Skip to content

Commit 363508d

Browse files
caoccaocaoccao
committed
🦄 refactor: Rewrite getV8HeapStatistics(), getV8HeapSpaceStatistics() v2
1 parent 1230da9 commit 363508d

File tree

7 files changed

+226
-89
lines changed

7 files changed

+226
-89
lines changed

‎cpp/jni/javet_monitor.cpp

+49-26
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,41 @@
2121

2222
namespace Javet {
2323
namespace Monitor {
24+
struct HeapSpaceStatisticsContainer {
25+
jobject allocationSpace;
26+
jobject completableFuture;
27+
28+
HeapSpaceStatisticsContainer(JNIEnv* jniEnv, jobject completableFuture, jobject allocationSpace) noexcept {
29+
this->allocationSpace = jniEnv->NewGlobalRef(allocationSpace);
30+
INCREASE_COUNTER(Javet::Monitor::CounterType::NewGlobalRef);
31+
this->completableFuture = jniEnv->NewGlobalRef(completableFuture);
32+
INCREASE_COUNTER(Javet::Monitor::CounterType::NewGlobalRef);
33+
}
34+
35+
~HeapSpaceStatisticsContainer() {
36+
FETCH_JNI_ENV(GlobalJavaVM);
37+
jniEnv->DeleteGlobalRef(allocationSpace);
38+
INCREASE_COUNTER(Javet::Monitor::CounterType::DeleteGlobalRef);
39+
jniEnv->DeleteGlobalRef(completableFuture);
40+
INCREASE_COUNTER(Javet::Monitor::CounterType::DeleteGlobalRef);
41+
}
42+
};
43+
44+
struct HeapStatisticsContainer {
45+
jobject completableFuture;
46+
47+
HeapStatisticsContainer(JNIEnv* jniEnv, jobject completableFuture) noexcept {
48+
this->completableFuture = jniEnv->NewGlobalRef(completableFuture);
49+
INCREASE_COUNTER(Javet::Monitor::CounterType::NewGlobalRef);
50+
}
51+
52+
~HeapStatisticsContainer() {
53+
FETCH_JNI_ENV(GlobalJavaVM);
54+
jniEnv->DeleteGlobalRef(completableFuture);
55+
INCREASE_COUNTER(Javet::Monitor::CounterType::DeleteGlobalRef);
56+
}
57+
};
58+
2459
void Initialize(JNIEnv* jniEnv) noexcept {
2560
jclassV8AllocationSpace = FIND_CLASS(jniEnv, "com/caoccao/javet/enums/V8AllocationSpace");
2661
jmethodIDV8AllocationSpaceGetIndex = jniEnv->GetMethodID(jclassV8AllocationSpace, "getIndex", "()I");
@@ -48,67 +83,57 @@ namespace Javet {
4883
v8::Isolate* v8Isolate,
4984
const jobject jAllocationSpace) noexcept {
5085
jobject jCompletableFuture = jniEnv->NewObject(jclassCompletableFuture, jmethodIDCompletableFutureConstructor);
51-
auto jobjectRefs = new jobject[]{ jniEnv->NewGlobalRef(jCompletableFuture), jniEnv->NewGlobalRef(jAllocationSpace) };
86+
auto containerPointer = new HeapSpaceStatisticsContainer(jniEnv, jCompletableFuture, jAllocationSpace);
5287
INCREASE_COUNTER(Javet::Monitor::CounterType::New);
53-
INCREASE_COUNTER(Javet::Monitor::CounterType::NewGlobalRef);
54-
INCREASE_COUNTER(Javet::Monitor::CounterType::NewGlobalRef);
5588
if (v8Isolate->IsInUse()) {
56-
v8Isolate->RequestInterrupt(GetHeapSpaceStatisticsCallback, &jobjectRefs);
89+
v8Isolate->RequestInterrupt(GetHeapSpaceStatisticsCallback, containerPointer);
5790
}
5891
else {
5992
auto v8Locker = v8::Locker(v8Isolate);
60-
GetHeapSpaceStatisticsCallback(v8Isolate, jobjectRefs);
93+
GetHeapSpaceStatisticsCallback(v8Isolate, containerPointer);
6194
}
6295
return jCompletableFuture;
6396
}
6497

6598
void GetHeapSpaceStatisticsCallback(v8::Isolate* v8Isolate, void* data) noexcept {
6699
FETCH_JNI_ENV(GlobalJavaVM);
67100
v8::HeapSpaceStatistics heapSpaceStatistics;
68-
auto jobjectRefs = static_cast<jobject*>(data);
69-
auto jCompletableFuture = jobjectRefs[0];
70-
auto jAllocationSpace = jobjectRefs[1];
71-
auto index = jniEnv->CallIntMethod(jAllocationSpace, jmethodIDV8AllocationSpaceGetIndex);
101+
auto containerPointer = static_cast<HeapSpaceStatisticsContainer*>(data);
102+
auto index = jniEnv->CallIntMethod(containerPointer->allocationSpace, jmethodIDV8AllocationSpaceGetIndex);
72103
v8Isolate->GetHeapSpaceStatistics(&heapSpaceStatistics, static_cast<size_t>(index));
73104
auto jHeapSpaceStatistics = jniEnv->NewObject(jclassV8HeapSpaceStatistics, jmethodIDV8HeapSpaceStatisticsConstructor,
74105
Javet::Converter::ToJavaString(jniEnv, heapSpaceStatistics.space_name()),
75106
static_cast<jlong>(heapSpaceStatistics.physical_space_size()),
76107
static_cast<jlong>(heapSpaceStatistics.space_available_size()),
77108
static_cast<jlong>(heapSpaceStatistics.space_size()),
78109
static_cast<jlong>(heapSpaceStatistics.space_used_size()));
79-
jniEnv->CallObjectMethod(jHeapSpaceStatistics, jmethodIDV8HeapSpaceStatisticsSetAllocationSpace, jAllocationSpace);
80-
jniEnv->CallBooleanMethod(jCompletableFuture, jmethodIDCompletableFutureComplete, jHeapSpaceStatistics);
110+
jniEnv->CallObjectMethod(jHeapSpaceStatistics, jmethodIDV8HeapSpaceStatisticsSetAllocationSpace, containerPointer->allocationSpace);
111+
jniEnv->CallBooleanMethod(containerPointer->completableFuture, jmethodIDCompletableFutureComplete, jHeapSpaceStatistics);
81112
jniEnv->DeleteLocalRef(jHeapSpaceStatistics);
82-
jniEnv->DeleteGlobalRef(jCompletableFuture);
83-
INCREASE_COUNTER(Javet::Monitor::CounterType::DeleteGlobalRef);
84-
jniEnv->DeleteGlobalRef(jAllocationSpace);
85-
INCREASE_COUNTER(Javet::Monitor::CounterType::DeleteGlobalRef);
86-
delete jobjectRefs;
113+
delete containerPointer;
87114
INCREASE_COUNTER(Javet::Monitor::CounterType::Delete);
88115
}
89116

90117
jobject GetHeapStatistics(
91118
JNIEnv* jniEnv,
92119
v8::Isolate* v8Isolate) noexcept {
93120
jobject jCompletableFuture = jniEnv->NewObject(jclassCompletableFuture, jmethodIDCompletableFutureConstructor);
94-
auto jobjectRefs = new jobject[]{ jniEnv->NewGlobalRef(jCompletableFuture) };
121+
auto containerPointer = new HeapStatisticsContainer(jniEnv, jCompletableFuture);
95122
INCREASE_COUNTER(Javet::Monitor::CounterType::New);
96-
INCREASE_COUNTER(Javet::Monitor::CounterType::NewGlobalRef);
97123
if (v8Isolate->IsInUse()) {
98-
v8Isolate->RequestInterrupt(GetHeapStatisticsCallback, jobjectRefs);
124+
v8Isolate->RequestInterrupt(GetHeapStatisticsCallback, containerPointer);
99125
}
100126
else {
101127
auto v8Locker = v8::Locker(v8Isolate);
102-
GetHeapStatisticsCallback(v8Isolate, jobjectRefs);
128+
GetHeapStatisticsCallback(v8Isolate, containerPointer);
103129
}
104130
return jCompletableFuture;
105131
}
106132

107133
void GetHeapStatisticsCallback(v8::Isolate* v8Isolate, void* data) noexcept {
108134
FETCH_JNI_ENV(GlobalJavaVM);
109135
v8::HeapStatistics heapStatistics;
110-
auto jobjectRefs = static_cast<jobject*>(data);
111-
auto jCompletableFuture = jobjectRefs[0];
136+
auto containerPointer = static_cast<HeapStatisticsContainer*>(data);
112137
v8Isolate->GetHeapStatistics(&heapStatistics);
113138
auto jHeapStatistics = jniEnv->NewObject(jclassV8HeapStatistics, jmethodIDV8HeapStatisticsConstructor,
114139
static_cast<jlong>(heapStatistics.does_zap_garbage()),
@@ -125,11 +150,9 @@ namespace Javet {
125150
static_cast<jlong>(heapStatistics.total_physical_size()),
126151
static_cast<jlong>(heapStatistics.used_global_handles_size()),
127152
static_cast<jlong>(heapStatistics.used_heap_size()));
128-
jniEnv->CallBooleanMethod(jCompletableFuture, jmethodIDCompletableFutureComplete, jHeapStatistics);
153+
jniEnv->CallBooleanMethod(containerPointer->completableFuture, jmethodIDCompletableFutureComplete, jHeapStatistics);
129154
jniEnv->DeleteLocalRef(jHeapStatistics);
130-
jniEnv->DeleteGlobalRef(jCompletableFuture);
131-
INCREASE_COUNTER(Javet::Monitor::CounterType::DeleteGlobalRef);
132-
delete jobjectRefs;
155+
delete containerPointer;
133156
INCREASE_COUNTER(Javet::Monitor::CounterType::Delete);
134157
}
135158

‎docs/reference/resource_management/memory_management.rst

+2-1
Original file line numberDiff line numberDiff line change
@@ -256,4 +256,5 @@ V8 exposes quite a few statistics for applications to analyze the memory usage,
256256

257257
.. note::
258258

259-
More statistics will be exposed in new releases. Please file issues if you need more of them.
259+
* If the ``V8Runtime`` is in use, calling ``getV8HeapSpaceStatistics()`` and ``getV8HeapStatistics()`` may take a slight chance (a race condition) to have tiny memory leak. Please refer to this `issue <https://issues.chromium.org/issues/345822325>`_ for details. It's recommended to call them when the ``V8Runtime`` is idle.
260+
* More statistics will be exposed in new releases. Please file issues if you need more of them.

‎docs/release_notes/release_notes_3_1.rst

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ Release Notes 3.1.x
66
--------------
77

88
* Rewrote ``getV8HeapStatistics()``, ``getV8HeapSpaceStatistics()`` for ``V8Runtime`` to remediate the racing condition
9+
* Added ``observerTimeoutMillis`` to ``JavetEngineConfig``
910

1011
3.1.3 V8 v12.6
1112
--------------

‎src/main/java/com/caoccao/javet/interop/engine/IJavetEnginePool.java

+5-3
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import com.caoccao.javet.enums.V8AllocationSpace;
2121
import com.caoccao.javet.exceptions.JavetException;
2222
import com.caoccao.javet.interfaces.IJavetClosable;
23-
import com.caoccao.javet.interop.V8Host;
2423
import com.caoccao.javet.interop.V8Runtime;
2524
import com.caoccao.javet.interop.engine.observers.*;
2625
import com.caoccao.javet.interop.monitoring.V8HeapSpaceStatistics;
@@ -77,7 +76,9 @@ default int getAverageReferenceCount() {
7776
*/
7877
default V8HeapSpaceStatistics getAverageV8HeapSpaceStatistics(final V8AllocationSpace v8AllocationSpace) {
7978
V8RuntimeObserverAverageV8HeapSpaceStatistics observer = new V8RuntimeObserverAverageV8HeapSpaceStatistics(
80-
v8AllocationSpace, getConfig().getPoolMaxSize());
79+
v8AllocationSpace,
80+
getConfig().getPoolMaxSize(),
81+
getConfig().getObserverTimeoutMillis());
8182
observe(observer);
8283
return observer.getResult();
8384
}
@@ -90,7 +91,8 @@ default V8HeapSpaceStatistics getAverageV8HeapSpaceStatistics(final V8Allocation
9091
*/
9192
default V8HeapStatistics getAverageV8HeapStatistics() {
9293
V8RuntimeObserverAverageV8HeapStatistics observer = new V8RuntimeObserverAverageV8HeapStatistics(
93-
getConfig().getPoolMaxSize());
94+
getConfig().getPoolMaxSize(),
95+
getConfig().getObserverTimeoutMillis());
9496
observe(observer);
9597
return observer.getResult();
9698
}

0 commit comments

Comments
 (0)