Skip to content

Commit 052b28e

Browse files
PanayotCankovPanayotCankov
committed
Apply some refactoring after PR reviews
1 parent 7092883 commit 052b28e

File tree

6 files changed

+79
-101
lines changed

6 files changed

+79
-101
lines changed

‎.gitignore‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,5 @@ bin/
2020
.project
2121
.DS_Store
2222
.settings
23+
24+
build-artifacts/

‎runtime/src/main/java/com/tns/AppConfig.java‎

Lines changed: 24 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,7 @@ private enum KnownKeys {
1818
MemoryCheckInterval("memoryCheckInterval", 0),
1919
FreeMemoryRatio("freeMemoryRatio", 0.0),
2020
Profiling("profiling", ""),
21-
MarkingMode("markingMode", "Full");
22-
23-
public static final KnownKeys[] asArray = {
24-
V8FlagsKey,
25-
CodeCacheKey,
26-
HeapSnapshotScriptKey,
27-
SnapshotFile,
28-
ProfilerOutputDirKey,
29-
GcThrottleTime,
30-
MemoryCheckInterval,
31-
FreeMemoryRatio,
32-
Profiling,
33-
MarkingMode
34-
};
21+
MarkingMode("markingMode", com.tns.MarkingMode.values()[0]);
3522

3623
private final String name;
3724
private final Object defaultValue;
@@ -44,19 +31,9 @@ private enum KnownKeys {
4431
public String getName() {
4532
return name;
4633
}
47-
4834
public Object getDefaultValue() {
4935
return defaultValue;
5036
}
51-
52-
int getIndex() {
53-
for (int i=0; i<asArray.length; i++) {
54-
if (asArray[i] == this) {
55-
return i;
56-
}
57-
}
58-
return -1;
59-
}
6037
}
6138

6239
private final static String AndroidKey = "android";
@@ -75,21 +52,21 @@ public AppConfig(File appDir) {
7552
try {
7653
rootObject = FileSystem.readJSONFile(packageInfo);
7754
if (rootObject != null) {
78-
if (rootObject.has(KnownKeys.Profiling.getName())) {
55+
if (rootObject.has(KnownKeys.Profiling.name())) {
7956
String profiling = rootObject.getString(KnownKeys.Profiling.getName());
80-
values[KnownKeys.Profiling.getIndex()] = profiling;
57+
values[KnownKeys.Profiling.ordinal()] = profiling;
8158
}
8259
if (rootObject.has(AndroidKey)) {
8360
JSONObject androidObject = rootObject.getJSONObject(AndroidKey);
8461
if (androidObject.has(KnownKeys.V8FlagsKey.getName())) {
85-
values[KnownKeys.V8FlagsKey.getIndex()] = androidObject.getString(KnownKeys.V8FlagsKey.getName());
62+
values[KnownKeys.V8FlagsKey.ordinal()] = androidObject.getString(KnownKeys.V8FlagsKey.getName());
8663
}
8764
if (androidObject.has(KnownKeys.CodeCacheKey.getName())) {
88-
values[KnownKeys.CodeCacheKey.getIndex()] = androidObject.getBoolean(KnownKeys.CodeCacheKey.getName());
65+
values[KnownKeys.CodeCacheKey.ordinal()] = androidObject.getBoolean(KnownKeys.CodeCacheKey.getName());
8966
}
9067
if (androidObject.has(KnownKeys.HeapSnapshotScriptKey.getName())) {
9168
String value = androidObject.getString(KnownKeys.HeapSnapshotScriptKey.getName());
92-
values[KnownKeys.HeapSnapshotScriptKey.getIndex()] = FileSystem.resolveRelativePath(appDir.getPath(), value, appDir + "/app/");
69+
values[KnownKeys.HeapSnapshotScriptKey.ordinal()] = FileSystem.resolveRelativePath(appDir.getPath(), value, appDir + "/app/");
9370
}
9471
if (androidObject.has(HeapSnapshotBlobKey)) {
9572
String value = androidObject.getString(HeapSnapshotBlobKey);
@@ -98,23 +75,27 @@ public AppConfig(File appDir) {
9875
if (dir.exists() && dir.isDirectory()) {
9976
// this path is expected to be a directory, containing three sub-directories: armeabi-v7a, x86 and arm64-v8a
10077
path = path + "/" + Build.CPU_ABI + "/" + KnownKeys.SnapshotFile.getName();
101-
values[KnownKeys.SnapshotFile.getIndex()] = path;
78+
values[KnownKeys.SnapshotFile.ordinal()] = path;
10279
}
10380
}
10481
if (androidObject.has(KnownKeys.ProfilerOutputDirKey.getName())) {
105-
values[KnownKeys.ProfilerOutputDirKey.getIndex()] = androidObject.getString(KnownKeys.ProfilerOutputDirKey.getName());
82+
values[KnownKeys.ProfilerOutputDirKey.ordinal()] = androidObject.getString(KnownKeys.ProfilerOutputDirKey.getName());
10683
}
10784
if (androidObject.has(KnownKeys.GcThrottleTime.getName())) {
108-
values[KnownKeys.GcThrottleTime.getIndex()] = androidObject.getInt(KnownKeys.GcThrottleTime.getName());
85+
values[KnownKeys.GcThrottleTime.ordinal()] = androidObject.getInt(KnownKeys.GcThrottleTime.getName());
10986
}
11087
if (androidObject.has(KnownKeys.MemoryCheckInterval.getName())) {
111-
values[KnownKeys.MemoryCheckInterval.getIndex()] = androidObject.getInt(KnownKeys.MemoryCheckInterval.getName());
88+
values[KnownKeys.MemoryCheckInterval.ordinal()] = androidObject.getInt(KnownKeys.MemoryCheckInterval.getName());
11289
}
11390
if (androidObject.has(KnownKeys.FreeMemoryRatio.getName())) {
114-
values[KnownKeys.FreeMemoryRatio.getIndex()] = androidObject.getDouble(KnownKeys.FreeMemoryRatio.getName());
91+
values[KnownKeys.FreeMemoryRatio.ordinal()] = androidObject.getDouble(KnownKeys.FreeMemoryRatio.getName());
11592
}
11693
if (androidObject.has(KnownKeys.MarkingMode.getName())) {
117-
values[KnownKeys.MarkingMode.getIndex()] = androidObject.getString(KnownKeys.MarkingMode.getName());
94+
try {
95+
String value = androidObject.getString(KnownKeys.MarkingMode.getName());
96+
values[KnownKeys.MarkingMode.ordinal()] = MarkingMode.valueOf(value);
97+
} catch(Throwable e) {
98+
}
11899
}
119100
}
120101
}
@@ -128,28 +109,27 @@ public Object[] getAsArray() {
128109
}
129110

130111
private static Object[] makeDefaultOptions() {
131-
Object[] result = new Object[KnownKeys.asArray.length];
132-
int index = 0;
133-
for (KnownKeys key: KnownKeys.asArray) {
134-
result[index++] = key.getDefaultValue();
112+
Object[] result = new Object[KnownKeys.values().length];
113+
for (KnownKeys key: KnownKeys.values()) {
114+
result[key.ordinal()] = key.getDefaultValue();
135115
}
136116
return result;
137117
}
138118

139119
public int getGcThrottleTime() {
140-
return (int)values[KnownKeys.GcThrottleTime.getIndex()];
120+
return (int)values[KnownKeys.GcThrottleTime.ordinal()];
141121
}
142122

143123
public int getMemoryCheckInterval() {
144-
return (int)values[KnownKeys.MemoryCheckInterval.getIndex()];
124+
return (int)values[KnownKeys.MemoryCheckInterval.ordinal()];
145125
}
146126

147127
public double getFreeMemoryRatio() {
148-
return (double)values[KnownKeys.FreeMemoryRatio.getIndex()];
128+
return (double)values[KnownKeys.FreeMemoryRatio.ordinal()];
149129
}
150130

151131
public String getProfilingMode() {
152-
return (String)values[KnownKeys.Profiling.getIndex()];
132+
return (String)values[KnownKeys.Profiling.ordinal()];
153133
}
154-
public String getMarkingMode() { return (String)values[KnownKeys.MarkingMode.getIndex()]; }
134+
public MarkingMode getMarkingMode() { return (MarkingMode)values[KnownKeys.MarkingMode.ordinal()]; }
155135
}

‎runtime/src/main/java/com/tns/MarkingMode.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package com.tns;
2+
3+
/**
4+
* Created by cankov on 24/08/2017.
5+
*/
6+
enum MarkingMode {
7+
full,
8+
none
9+
}

‎runtime/src/main/java/com/tns/Runtime.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -202,17 +202,17 @@ public DynamicConfiguration getDynamicConfig() {
202202
return dynamicConfig;
203203
}
204204

205-
public int getMarkingMode() {
206-
String mode = staticConfiguration == null || staticConfiguration.appConfig == null ? "full" : staticConfiguration.appConfig.getMarkingMode();
207-
if ("none".equals(mode)) {
208-
return 1;
205+
@RuntimeCallable
206+
public int getMarkingModeOrdinal() {
207+
if (staticConfiguration != null && staticConfiguration.appConfig != null) {
208+
return staticConfiguration.appConfig.getMarkingMode().ordinal();
209+
} else {
210+
return MarkingMode.values()[0].ordinal();
209211
}
210-
return 0;
211212
}
212213

213214
public static boolean isInitialized() {
214215
Runtime runtime = Runtime.getCurrentRuntime();
215-
216216
return (runtime != null) ? runtime.isInitializedImpl() : false;
217217
}
218218

‎runtime/src/main/jni/ObjectManager.cpp‎

Lines changed: 31 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,7 @@ ObjectManager::ObjectManager(jobject javaRuntimeObject) :
2222
m_env(JEnv()),
2323
m_numberOfGC(0),
2424
m_currentObjectId(0),
25-
m_cache(NewWeakGlobalRefCallback, DeleteWeakGlobalRefCallback, 1000, this),
26-
m_markingMode(JavaScriptMarkingMode::Full) {
25+
m_cache(NewWeakGlobalRefCallback, DeleteWeakGlobalRefCallback, 1000, this) {
2726

2827
auto runtimeClass = m_env.FindClass("com/tns/Runtime");
2928
assert(runtimeClass != nullptr);
@@ -55,13 +54,9 @@ ObjectManager::ObjectManager(jobject javaRuntimeObject) :
5554
auto useGlobalRefs = m_env.CallStaticBooleanMethod(runtimeClass, useGlobalRefsMethodID);
5655
m_useGlobalRefs = useGlobalRefs == JNI_TRUE;
5756

58-
auto getMarkingModeMethodID = m_env.GetMethodID(runtimeClass, "getMarkingMode", "()I");
59-
jint markingMode = m_env.CallIntMethod(m_javaRuntimeObject, getMarkingModeMethodID);
60-
switch(markingMode) {
61-
case 1:
62-
m_markingMode = JavaScriptMarkingMode::None;
63-
break;
64-
}
57+
auto getMarkingModeOrdinalMethodID = m_env.GetMethodID(runtimeClass, "getMarkingModeOrdinal", "()I");
58+
jint markingMode = m_env.CallIntMethod(m_javaRuntimeObject, getMarkingModeOrdinalMethodID);
59+
m_markingMode = static_cast<JavaScriptMarkingMode>(markingMode);
6560
}
6661

6762
void ObjectManager::SetInstanceIsolate(Isolate* isolate) {
@@ -99,32 +94,35 @@ JniLocalRef ObjectManager::GetJavaObjectByJsObject(const Local<Object>& object)
9994

10095
ObjectManager::JSInstanceInfo* ObjectManager::GetJSInstanceInfo(const Local<Object>& object) {
10196
JSInstanceInfo* jsInstanceInfo = nullptr;
97+
if (IsJsRuntimeObject(object)) {
98+
return GetJSInstanceInfoFromRuntimeObject(object);
99+
}
100+
return nullptr;
101+
}
102102

103-
auto isolate = m_isolate;
104-
HandleScope handleScope(isolate);
103+
ObjectManager::JSInstanceInfo* ObjectManager::GetJSInstanceInfoFromRuntimeObject(const Local<Object>& object) {
104+
HandleScope handleScope(m_isolate);
105105

106-
if (IsJsRuntimeObject(object)) {
107-
const int jsInfoIdx = static_cast<int>(MetadataNodeKeys::JsInfo);
108-
auto jsInfo = object->GetInternalField(jsInfoIdx);
109-
if (jsInfo->IsUndefined()) {
110-
//Typescript object layout has an object instance as child of the actual registered instance. checking for that
111-
auto prototypeObject = object->GetPrototype().As<Object>();
112-
113-
if (!prototypeObject.IsEmpty() && prototypeObject->IsObject()) {
114-
DEBUG_WRITE("GetJSInstanceInfo: need to check prototype :%d", prototypeObject->GetIdentityHash());
115-
if (IsJsRuntimeObject(prototypeObject)) {
116-
jsInfo = prototypeObject->GetInternalField(jsInfoIdx);
117-
}
106+
const int jsInfoIdx = static_cast<int>(MetadataNodeKeys::JsInfo);
107+
auto jsInfo = object->GetInternalField(jsInfoIdx);
108+
if (jsInfo->IsUndefined()) {
109+
//Typescript object layout has an object instance as child of the actual registered instance. checking for that
110+
auto prototypeObject = object->GetPrototype().As<Object>();
111+
112+
if (!prototypeObject.IsEmpty() && prototypeObject->IsObject()) {
113+
DEBUG_WRITE("GetJSInstanceInfo: need to check prototype :%d", prototypeObject->GetIdentityHash());
114+
if (IsJsRuntimeObject(prototypeObject)) {
115+
jsInfo = prototypeObject->GetInternalField(jsInfoIdx);
118116
}
119117
}
118+
}
120119

121-
if (!jsInfo.IsEmpty() && jsInfo->IsExternal()) {
122-
auto external = jsInfo.As<External>();
123-
jsInstanceInfo = static_cast<JSInstanceInfo*>(external->Value());
124-
}
120+
if (!jsInfo.IsEmpty() && jsInfo->IsExternal()) {
121+
auto external = jsInfo.As<External>();
122+
return static_cast<JSInstanceInfo*>(external->Value());
125123
}
126124

127-
return jsInstanceInfo;
125+
return nullptr;
128126
}
129127

130128
bool ObjectManager::IsJsRuntimeObject(const v8::Local<v8::Object>& object) {
@@ -297,41 +295,25 @@ void ObjectManager::JSObjectFinalizerStatic(const WeakCallbackInfo<ObjectWeakCal
297295
}
298296

299297
void ObjectManager::JSObjectFinalizer(Isolate* isolate, ObjectWeakCallbackState* callbackState) {
298+
HandleScope handleScope(m_isolate);
300299
Persistent<Object>* po = callbackState->target;
300+
auto jsInstanceInfo = GetJSInstanceInfoFromRuntimeObject(po->Get(m_isolate));
301301

302-
auto jsInfoIdx = static_cast<int>(MetadataNodeKeys::JsInfo);
303-
auto jsInstance = po->Get(m_isolate);
304-
auto jsInfo = jsInstance->GetInternalField(jsInfoIdx);
305-
if (jsInfo->IsUndefined()) {
306-
// Typescript object layout has an object instance as child of the actual registered instance. checking for that
307-
auto prototypeObject = jsInstance->GetPrototype().As<Object>();
308-
if (!prototypeObject.IsEmpty() && prototypeObject->IsObject()) {
309-
DEBUG_WRITE("GetJSInstanceInfo: need to check prototype :%d", prototypeObject->GetIdentityHash());
310-
if (IsJsRuntimeObject(prototypeObject)) {
311-
jsInfo = prototypeObject->GetInternalField(jsInfoIdx);
312-
}
313-
}
314-
}
315-
316-
if (jsInfo.IsEmpty() || !jsInfo->IsExternal()) {
317-
// The JavaScript instance has been forcefully disconnected from the Java instance.
302+
if (!jsInstanceInfo) {
318303
po->Reset();
319304
return;
320305
}
321306

322-
auto external = jsInfo.As<External>();
323-
auto jsInstanceInfo = static_cast<JSInstanceInfo *>(external->Value());
324307
auto javaObjectID = jsInstanceInfo->JavaObjectID;
325-
326308
jboolean isJavaInstanceAlive = m_env.CallBooleanMethod(m_javaRuntimeObject, MAKE_INSTANCE_WEAK_AND_CHECK_IF_ALIVE_METHOD_ID, javaObjectID);
327309
if (isJavaInstanceAlive) {
328310
// If the Java instance is alive, keep the JavaScript instance alive.
329-
// TODO: Check should we really register the finalizer again?
330311
po->SetWeak(callbackState, JSObjectFinalizerStatic, WeakCallbackType::kFinalizer);
331312
} else {
332313
// If the Java instance is dead, this JavaScript instance can be let die.
333314
delete jsInstanceInfo;
334-
jsInstance->SetInternalField(jsInfoIdx, Undefined(m_isolate));
315+
auto jsInfoIdx = static_cast<int>(MetadataNodeKeys::JsInfo);
316+
po->Get(m_isolate)->SetInternalField(jsInfoIdx, Undefined(m_isolate));
335317
po->Reset();
336318
}
337319
}

‎runtime/src/main/jni/ObjectManager.h‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,13 @@ class ObjectManager {
5757
END
5858
};
5959

60-
enum class JavaScriptMarkingMode {
60+
/**
61+
* Memory management modes. Keep the members in sync with the java/com/tns/MarkingMode.
62+
*/
63+
enum JavaScriptMarkingMode {
6164
/**
6265
* For JavaScript instances with implementation objects that were marked for collection,
63-
* MarkReachableObjects will scann the whole graph of reachable objects and keep strong reference to
66+
* MarkReachableObjects will scan the whole graph of reachable objects and keep strong reference to
6467
* the Java instances of implementation objects.
6568
*/
6669
Full,
@@ -140,6 +143,8 @@ class ObjectManager {
140143

141144
JSInstanceInfo* GetJSInstanceInfo(const v8::Local<v8::Object>& object);
142145

146+
JSInstanceInfo* GetJSInstanceInfoFromRuntimeObject(const v8::Local<v8::Object>& object);
147+
143148
void ReleaseJSInstance(v8::Persistent<v8::Object>* po, JSInstanceInfo* jsInstanceInfo);
144149

145150
void ReleaseRegularObjects();

0 commit comments

Comments
 (0)