Skip to content

Commit fbad2b7

Browse files
committed
refactor: remove the markingMode "full" machinery
Only markingMode "none" is supported. "full" was already non-functional: its weak callback queued wrappers for processing that no longer existed, so they were revived forever. - ObjectManager always arms the finalizer path; the full-mode weak callback, its GC bookkeeping, the batch DirectBuffer plumbing and the unused declarations behind it are gone, as are DirectBuffer itself and the Java makeInstanceWeak(ByteBuffer)/checkWeakObjectAreAlive/getMarkingModeOrdinal. - AppConfig drops the key (native config indices shift accordingly) and logs a warning when an app still sets markingMode to anything but "none". - The __markingMode global is gone; testGC.js no longer gates on it, so its specs now run (they pass under the finalizer path).
1 parent a7999ac commit fbad2b7

11 files changed

Lines changed: 11 additions & 481 deletions

File tree

‎test-app/app/src/main/assets/app/package.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
"gcThrottleTime": 500,
77
"memoryCheckInterval": 10,
88
"freeMemoryRatio": 0.50,
9-
"markingMode": "none",
109
"maxLogcatObjectSize": 1024,
1110
"forceLog": false,
1211
"suppressCallJSMethodExceptions": false,

‎test-app/app/src/main/assets/app/tests/testGC.js‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
1-
// Run GC tests only in Full marking mode
2-
let describeFunc = __markingMode == 0 ? describe : xdescribe;
3-
4-
describeFunc("Tests garbage collection", function () {
1+
describe("Tests garbage collection", function () {
52
var myCustomEquality = function(first, second) {
63
return first == second;
74
};

‎test-app/runtime/CMakeLists.txt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,6 @@ add_library(
182182
src/main/cpp/ConcurrentQueue.cpp
183183
src/main/cpp/Constants.cpp
184184
src/main/cpp/CrashBreadcrumbs.cpp
185-
src/main/cpp/DirectBuffer.cpp
186185
src/main/cpp/ErrorEvents.cpp
187186
src/main/cpp/EventLoop.cpp
188187
src/main/cpp/Events.cpp

‎test-app/runtime/src/main/cpp/DirectBuffer.cpp‎

Lines changed: 0 additions & 56 deletions
This file was deleted.

‎test-app/runtime/src/main/cpp/DirectBuffer.h‎

Lines changed: 0 additions & 30 deletions
This file was deleted.

‎test-app/runtime/src/main/cpp/ObjectManager.cpp‎

Lines changed: 3 additions & 191 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ using namespace tns;
2020

2121
ObjectManager::ObjectManager(jobject javaRuntimeObject)
2222
: m_javaRuntimeObject(javaRuntimeObject),
23-
m_numberOfGC(0),
2423
m_currentObjectId(0),
2524
m_cache(NewWeakGlobalRefCallback, DeleteWeakGlobalRefCallback,
2625
ValidateWeakGlobalRefCallback, 1000, this) {
@@ -37,12 +36,6 @@ ObjectManager::ObjectManager(jobject javaRuntimeObject)
3736
auto useGlobalRefs =
3837
env.CallStaticBooleanMethod(runtimeClass, useGlobalRefsMethodID);
3938
m_useGlobalRefs = useGlobalRefs == JNI_TRUE;
40-
41-
auto getMarkingModeOrdinalMethodID =
42-
env.GetMethodID(runtimeClass, "getMarkingModeOrdinal", "()I");
43-
jint markingMode =
44-
env.CallIntMethod(m_javaRuntimeObject, getMarkingModeOrdinalMethodID);
45-
m_markingMode = static_cast<JavaScriptMarkingMode>(markingMode);
4639
}
4740

4841
void ObjectManager::InitializeJNI() {
@@ -60,10 +53,6 @@ void ObjectManager::InitializeJNI() {
6053
runtimeClass, "getOrCreateJavaObjectID", "(Ljava/lang/Object;)I");
6154
NS_CHECK(GET_OR_CREATE_JAVA_OBJECT_ID_METHOD_ID != nullptr);
6255

63-
MAKE_INSTANCE_WEAK_BATCH_METHOD_ID = env.GetMethodID(
64-
runtimeClass, "makeInstanceWeak", "(Ljava/nio/ByteBuffer;IZ)V");
65-
NS_CHECK(MAKE_INSTANCE_WEAK_BATCH_METHOD_ID != nullptr);
66-
6756
MAKE_INSTANCE_WEAK_AND_CHECK_IF_ALIVE_METHOD_ID =
6857
env.GetMethodID(runtimeClass, "makeInstanceWeakAndCheckIfAlive", "(I)Z");
6958
NS_CHECK(MAKE_INSTANCE_WEAK_AND_CHECK_IF_ALIVE_METHOD_ID != nullptr);
@@ -72,11 +61,6 @@ void ObjectManager::InitializeJNI() {
7261
env.GetMethodID(runtimeClass, "releaseNativeCounterpart", "(I)V");
7362
NS_CHECK(RELEASE_NATIVE_INSTANCE_METHOD_ID != nullptr);
7463

75-
CHECK_WEAK_OBJECTS_ARE_ALIVE_METHOD_ID =
76-
env.GetMethodID(runtimeClass, "checkWeakObjectAreAlive",
77-
"(Ljava/nio/ByteBuffer;Ljava/nio/ByteBuffer;I)V");
78-
NS_CHECK(CHECK_WEAK_OBJECTS_ARE_ALIVE_METHOD_ID != nullptr);
79-
8064
JAVA_LANG_CLASS = env.FindClass("java/lang/Class");
8165
NS_CHECK(JAVA_LANG_CLASS != nullptr);
8266

@@ -328,19 +312,14 @@ void ObjectManager::Link(const Local<Object>& object, uint32_t javaObjectID,
328312
object->GetIdentityHash(), javaObjectID);
329313

330314
auto jsInstanceInfo =
331-
new JSInstanceInfo(false /*isJavaObjWeak*/, javaObjectID, clazz);
315+
new JSInstanceInfo(javaObjectID, clazz);
332316

333317
auto objectHandle = new Persistent<Object>(isolate, object);
334318
auto state = new ObjectWeakCallbackState(this, jsInstanceInfo, objectHandle);
335319

336320
// subscribe for JS GC event
337-
if (m_markingMode == JavaScriptMarkingMode::None) {
338-
objectHandle->SetWeak(state, JSObjectFinalizerStatic,
339-
WeakCallbackType::kFinalizer);
340-
} else {
341-
objectHandle->SetWeak(state, JSObjectWeakCallbackStatic,
342-
WeakCallbackType::kFinalizer);
343-
}
321+
objectHandle->SetWeak(state, JSObjectFinalizerStatic,
322+
WeakCallbackType::kFinalizer);
344323

345324
auto jsInfoIdx = static_cast<int>(MetadataNodeKeys::JsInfo);
346325

@@ -390,17 +369,6 @@ string ObjectManager::GetClassName(jclass clazz) {
390369
return className;
391370
}
392371

393-
void ObjectManager::JSObjectWeakCallbackStatic(
394-
const WeakCallbackInfo<ObjectWeakCallbackState>& data) {
395-
ObjectWeakCallbackState* callbackState = data.GetParameter();
396-
397-
ObjectManager* thisPtr = callbackState->thisPtr;
398-
399-
auto isolate = data.GetIsolate();
400-
401-
thisPtr->JSObjectWeakCallback(isolate, callbackState);
402-
}
403-
404372
void ObjectManager::JSObjectFinalizerStatic(
405373
const WeakCallbackInfo<ObjectWeakCallbackState>& data) {
406374
ObjectWeakCallbackState* callbackState = data.GetParameter();
@@ -449,68 +417,6 @@ void ObjectManager::JSObjectFinalizer(Isolate* isolate,
449417
}
450418
}
451419

452-
/*
453-
* When JS GC happens change state of the java counterpart to mirror state of JS
454-
* object and REVIVE the JS object unconditionally "Regular" js objects are
455-
* pushed into the "regular objects" array "Callback" js objects (ones that have
456-
* implementation object) are pushed into two "special objects" array: -ones
457-
* called for the first time which are originally strong in java -ones called
458-
* for the second or next time which are already weak in java too These objects
459-
* are categorized by "regular" and "callback" and saved in different arrays for
460-
* performance optimizations during GC
461-
* */
462-
void ObjectManager::JSObjectWeakCallback(
463-
Isolate* isolate, ObjectWeakCallbackState* callbackState) {
464-
HandleScope handleScope(isolate);
465-
466-
Persistent<Object>* po = callbackState->target;
467-
468-
auto itFound = m_visitedPOs.find(po);
469-
470-
if (itFound == m_visitedPOs.end()) {
471-
m_visitedPOs.insert(po);
472-
473-
auto obj = Local<Object>::New(isolate, *po);
474-
JSInstanceInfo* jsInstanceInfo = GetJSInstanceInfo(obj);
475-
476-
if (jsInstanceInfo != nullptr) {
477-
int javaObjectID = jsInstanceInfo->JavaObjectID;
478-
479-
bool hasImplObj = HasImplObject(isolate, obj);
480-
481-
DEBUG_WRITE("JSObjectWeakCallback objectId: %d, hasImplObj=%d",
482-
javaObjectID, hasImplObj);
483-
484-
if (hasImplObj) {
485-
if (jsInstanceInfo->IsJavaObjectWeak) {
486-
m_implObjWeak.emplace_back(po, javaObjectID);
487-
} else {
488-
m_implObjStrong.emplace(javaObjectID, po);
489-
jsInstanceInfo->IsJavaObjectWeak = true;
490-
}
491-
} else {
492-
if (m_markedForGC.empty()) {
493-
// Emulates the behavior in the OnGcStarted callback. Тhis is
494-
// necessary as the hooking to the V8 GC is done only on the
495-
// markSweepCompact phase. The JSObjectWeakCallback however is still
496-
// triggered in other V8 GC phases (scavenger for example). This
497-
// creates a problem that there is no 'top' on the m_markedForGC
498-
// stack.
499-
GarbageCollectionInfo gcInfo(++m_numberOfGC);
500-
gcInfo.markedForGC.push_back(po);
501-
m_markedForGC.push(gcInfo);
502-
} else {
503-
auto& topGCInfo = m_markedForGC.top();
504-
topGCInfo.markedForGC.push_back(po);
505-
}
506-
}
507-
}
508-
}
509-
510-
po->SetWeak(callbackState, JSObjectWeakCallbackStatic,
511-
WeakCallbackType::kFinalizer);
512-
}
513-
514420
int ObjectManager::GenerateNewObjectID() {
515421
const int one = 1;
516422
int oldValue = __sync_fetch_and_add(&m_currentObjectId, one);
@@ -562,94 +468,6 @@ ObjectManager::~ObjectManager() {
562468
m_cache.clear();
563469
}
564470

565-
void ObjectManager::ReleaseJSInstance(Persistent<Object>* po,
566-
JSInstanceInfo* jsInstanceInfo) {
567-
int javaObjectID = jsInstanceInfo->JavaObjectID;
568-
569-
auto it = m_idToObject.find(javaObjectID);
570-
571-
if (it == m_idToObject.end()) {
572-
stringstream ss;
573-
ss << "(InternalError): Js object with id: " << javaObjectID
574-
<< " not found";
575-
throw NativeScriptException(ss.str());
576-
}
577-
578-
NS_DCHECK(po == it->second->target);
579-
580-
ObjectWeakCallbackState* callbackState = it->second;
581-
m_idToObject.erase(it);
582-
delete callbackState;
583-
m_released.insert(po, javaObjectID);
584-
po->Reset();
585-
586-
delete po;
587-
delete jsInstanceInfo;
588-
589-
DEBUG_WRITE("ReleaseJSObject instance disposed. id:%d", javaObjectID);
590-
}
591-
592-
/*
593-
* The "regular" JS objects added on ObjectManager::JSObjectWeakCallback are
594-
* dealt with(released) here.
595-
* */
596-
void ObjectManager::ReleaseRegularObjects() {
597-
TNSPERF();
598-
599-
HandleScope handleScope(m_isolate);
600-
601-
auto propName =
602-
String::NewFromUtf8(m_isolate, "t::gcNum", NewStringType::kNormal)
603-
.ToLocalChecked();
604-
605-
auto& topGCInfo = m_markedForGC.top();
606-
auto& marked = topGCInfo.markedForGC;
607-
int numberOfGC = topGCInfo.numberOfGC;
608-
609-
for (auto po : marked) {
610-
if (m_released.contains(po)) {
611-
continue;
612-
}
613-
614-
auto obj = Local<Object>::New(m_isolate, *po);
615-
616-
NS_DCHECK(!obj.IsEmpty());
617-
618-
Local<Value> gcNum;
619-
V8GetPrivateValue(m_isolate, obj, propName, gcNum);
620-
621-
bool isReachableFromImplementationObject = false;
622-
623-
if (!gcNum.IsEmpty() && gcNum->IsNumber()) {
624-
double objGcNum = gcNum.As<Number>()->Value();
625-
626-
// done so we can release only java objects from this GC stack and pass
627-
// all objects that will be released in parent GC stacks
628-
isReachableFromImplementationObject = objGcNum >= numberOfGC;
629-
}
630-
631-
JSInstanceInfo* jsInstanceInfo = GetJSInstanceInfo(obj);
632-
633-
if (!isReachableFromImplementationObject) {
634-
if (!jsInstanceInfo->IsJavaObjectWeak) {
635-
jsInstanceInfo->IsJavaObjectWeak = true;
636-
637-
ReleaseJSInstance(po, jsInstanceInfo);
638-
}
639-
}
640-
}
641-
642-
marked.clear();
643-
}
644-
645-
bool ObjectManager::HasImplObject(Isolate* isolate, const Local<Object>& obj) {
646-
auto implObject = MetadataNode::GetImplementationObject(isolate, obj);
647-
648-
bool hasImplObj = !implObject.IsEmpty();
649-
650-
return hasImplObj;
651-
}
652-
653471
jweak ObjectManager::NewWeakGlobalRefCallback(const int& javaObjectID,
654472
void* state) {
655473
auto objManager = reinterpret_cast<ObjectManager*>(state);
@@ -755,16 +573,10 @@ void ObjectManager::ReleaseNativeCounterpart(v8::Local<v8::Object>& object) {
755573
}
756574
}
757575

758-
ObjectManager::JavaScriptMarkingMode ObjectManager::GetMarkingMode() {
759-
return this->m_markingMode;
760-
}
761-
762576
jclass ObjectManager::JAVA_LANG_CLASS = nullptr;
763577
jmethodID ObjectManager::GET_NAME_METHOD_ID = nullptr;
764578
jmethodID ObjectManager::GET_JAVAOBJECT_BY_ID_METHOD_ID = nullptr;
765579
jmethodID ObjectManager::GET_OR_CREATE_JAVA_OBJECT_ID_METHOD_ID = nullptr;
766-
jmethodID ObjectManager::MAKE_INSTANCE_WEAK_BATCH_METHOD_ID = nullptr;
767580
jmethodID ObjectManager::MAKE_INSTANCE_WEAK_AND_CHECK_IF_ALIVE_METHOD_ID =
768581
nullptr;
769582
jmethodID ObjectManager::RELEASE_NATIVE_INSTANCE_METHOD_ID = nullptr;
770-
jmethodID ObjectManager::CHECK_WEAK_OBJECTS_ARE_ALIVE_METHOD_ID = nullptr;

0 commit comments

Comments
 (0)