From 2338a3e46ca5cf49d3f046db78628ffbba7e97a5 Mon Sep 17 00:00:00 2001 From: Jack Lu Date: Wed, 25 Sep 2024 17:19:54 -0400 Subject: [PATCH] Address review comments Add assertion to j9gc_ext calls Update formatting Signed-off-by: Jack Lu --- runtime/gc_base/ReferenceChainWalker.cpp | 15 ++++++------ runtime/gc_base/ReferenceChainWalker.hpp | 5 ++-- runtime/gc_include/HeapIteratorAPI.h | 4 ++-- runtime/jvmti/jvmtiHeap.c | 30 ++++++++++++------------ runtime/jvmti/jvmtiHeap10.c | 29 +++++++++++------------ 5 files changed, 42 insertions(+), 41 deletions(-) diff --git a/runtime/gc_base/ReferenceChainWalker.cpp b/runtime/gc_base/ReferenceChainWalker.cpp index 984fc5cebec..537cfc43ddb 100644 --- a/runtime/gc_base/ReferenceChainWalker.cpp +++ b/runtime/gc_base/ReferenceChainWalker.cpp @@ -80,6 +80,7 @@ j9gc_ext_reachable_objects_do( uintptr_t walkFlags) { MM_EnvironmentBase *env = MM_EnvironmentBase::getEnvironment(vmThread->omrVMThread); + Assert_MM_mustHaveExclusiveVMAccess(env->getOmrVMThread()); /* Make sure the heap is walkable (flush TLH's, secure heap integrity) */ vmThread->javaVM->memoryManagerFunctions->j9gc_flush_caches_for_walk(vmThread->javaVM); @@ -117,6 +118,7 @@ j9gc_ext_reachable_from_object_do( uintptr_t walkFlags) { MM_EnvironmentBase *env = MM_EnvironmentBase::getEnvironment(vmThread->omrVMThread); + Assert_MM_mustHaveExclusiveVMAccess(env->getOmrVMThread()); /* Make sure the heap is walkable (flush TLH's, secure heap integrity) */ vmThread->javaVM->memoryManagerFunctions->j9gc_flush_caches_for_walk(vmThread->javaVM); @@ -661,7 +663,7 @@ MM_ReferenceChainWalker::doStackSlot(J9Object **slotPtr, void *walkState, const ((J9StackWalkState *)walkState)->walkThread->threadObject = _vThreadObject; } #endif /* JAVA_SPEC_VERSION >= 19 */ - J9MM_StackSlotDescriptor stackSlotDescriptor = { ((J9StackWalkState *)walkState)->walkThread, (J9StackWalkState *)walkState }; + J9MM_StackSlotDescriptor stackSlotDescriptor = {((J9StackWalkState *)walkState)->walkThread, (J9StackWalkState *)walkState}; if (J9_STACKWALK_SLOT_TYPE_JNI_LOCAL == ((J9StackWalkState *)walkState)->slotType) { doSlot(slotPtr, J9GC_ROOT_TYPE_JNI_LOCAL, -1, (J9Object *)&stackSlotDescriptor); } else { @@ -693,12 +695,11 @@ MM_ReferenceChainWalker::doVMThreadSlot(J9Object **slotPtr, GC_VMThreadIterator case vmthreaditerator_state_slots: doSlot(slotPtr, J9GC_ROOT_TYPE_THREAD_SLOT, -1, NULL); break; - case vmthreaditerator_state_jni_slots: - { - J9MM_StackSlotDescriptor stackSlotDescriptor = {vmThreadIterator->getVMThread(), NULL}; - doSlot(slotPtr, J9GC_ROOT_TYPE_JNI_LOCAL, -1, (J9Object*)&stackSlotDescriptor); - break; - } + case vmthreaditerator_state_jni_slots: { + J9MM_StackSlotDescriptor stackSlotDescriptor = {vmThreadIterator->getVMThread(), NULL}; + doSlot(slotPtr, J9GC_ROOT_TYPE_JNI_LOCAL, -1, (J9Object *)&stackSlotDescriptor); + break; + } #if defined(J9VM_INTERP_HOT_CODE_REPLACEMENT) case vmthreaditerator_state_monitor_records: if (isHeapObject(slotValue) && !_heap->objectIsInGap(slotValue)) { diff --git a/runtime/gc_base/ReferenceChainWalker.hpp b/runtime/gc_base/ReferenceChainWalker.hpp index 90bb111d608..dd41009d095 100644 --- a/runtime/gc_base/ReferenceChainWalker.hpp +++ b/runtime/gc_base/ReferenceChainWalker.hpp @@ -75,8 +75,8 @@ class MM_ReferenceChainWalker : public MM_RootScanner bool _isTerminating; /**< Set when no more callbacks should be queued */ bool _shouldPreindexInterfaceFields; /**< if true, indexes interface fields of the class being visited before class and superclass fields, otherwise, returns them in the order they appear in an object instance (CMVC 142897) */ #if JAVA_SPEC_VERSION >= 19 - bool _includeVThreadObject; /**< Set when VirtualThread object is needed by callback function. */ - J9Object *_vThreadObject; /**< Cached object ref of VirtualThread object. */ + bool _includeVThreadObject; /**< Set when VirtualThread object is needed by callback function */ + J9Object *_vThreadObject; /**< Cached object ref of VirtualThread object */ #endif /* JAVA_SPEC_VERSION >= 19 */ MM_ReferenceChainWalkerMarkMap *_markMap; /**< Mark Map created for Reference Chain Walker */ MM_Heap *_heap; /**< Cached pointer to the heap */ @@ -271,6 +271,7 @@ class MM_ReferenceChainWalker : public MM_RootScanner #if JAVA_SPEC_VERSION >= 19 void includeVThreadObject() { _includeVThreadObject = true; } #endif /* JAVA_SPEC_VERSION >= 19 */ + /** * Added to support bi-modal interface indexing in JVMTI (CMVC 142897). * Detail: heap10 requires no pre-indexing in order to preserve Java5 behaviour but heap11 requires pre-indexing to pass a Java6 JCK diff --git a/runtime/gc_include/HeapIteratorAPI.h b/runtime/gc_include/HeapIteratorAPI.h index db223d01c03..4246eef6a55 100644 --- a/runtime/gc_include/HeapIteratorAPI.h +++ b/runtime/gc_include/HeapIteratorAPI.h @@ -165,8 +165,8 @@ typedef struct J9MM_HeapRootSlotDescriptor { } J9MM_HeapRootSlotDescriptor; typedef struct J9MM_StackSlotDescriptor { - J9VMThread *vmThread; /**< Thread containing the stack slot. */ - J9StackWalkState *walkState; /**< WalkState that found the slot, or null if found directly in thread local tables. */ + J9VMThread *vmThread; /**< Thread containing the stack slot */ + J9StackWalkState *walkState; /**< WalkState that found the slot, or null if found directly in thread local tables */ } J9MM_StackSlotDescriptor; typedef jvmtiIterationControl (*rootIteratorCallBackFunc)(void* ptr, J9MM_HeapRootSlotDescriptor *rootDesc, void *userData); diff --git a/runtime/jvmti/jvmtiHeap.c b/runtime/jvmti/jvmtiHeap.c index cb07e7c7ee5..38f1e611537 100644 --- a/runtime/jvmti/jvmtiHeap.c +++ b/runtime/jvmti/jvmtiHeap.c @@ -840,16 +840,16 @@ wrap_heapReferenceCallback(J9JavaVM * vm, J9JVMTIHeapData * iteratorData) * */ static UDATA -jvmtiHeapFollowRefs_getStackData(J9JVMTIHeapData * iteratorData, J9MM_StackSlotDescriptor *stackSlotDescriptor) +jvmtiHeapFollowRefs_getStackData(J9JVMTIHeapData *iteratorData, J9MM_StackSlotDescriptor *stackSlotDescriptor) { J9JVMTIHeapEvent * e = &iteratorData->event; jint slot = -1; jint depth = -1; J9Method *ramMethod = NULL; jmethodID method = (jmethodID) -1; - J9JVMTIObjectTag search; - J9JVMTIObjectTag *result; - jlong threadID; + J9JVMTIObjectTag search = {NULL, 0}; + J9JVMTIObjectTag *result = NULL; + jlong threadID = 0; J9StackWalkState *walkState = stackSlotDescriptor->walkState; if (NULL != walkState) { @@ -860,15 +860,15 @@ jvmtiHeapFollowRefs_getStackData(J9JVMTIHeapData * iteratorData, J9MM_StackSlotD /* Convert internal slot type to JVMTI type */ switch (walkState->slotType) { - case J9_STACKWALK_SLOT_TYPE_JNI_LOCAL: - e->refKind = JVMTI_HEAP_REFERENCE_JNI_LOCAL; - break; - case J9_STACKWALK_SLOT_TYPE_METHOD_LOCAL: - e->refKind = JVMTI_HEAP_REFERENCE_STACK_LOCAL; - break; - default: - /* Do not callback with stack slot types other then JNI or STACK Local */ - return 0; + case J9_STACKWALK_SLOT_TYPE_JNI_LOCAL: + e->refKind = JVMTI_HEAP_REFERENCE_JNI_LOCAL; + break; + case J9_STACKWALK_SLOT_TYPE_METHOD_LOCAL: + e->refKind = JVMTI_HEAP_REFERENCE_STACK_LOCAL; + break; + default: + /* Do not callback with stack slot types other then JNI or STACK Local. */ + return 0; } /* If there's no method, slot, method and depth are set to -1 by default. */ @@ -889,10 +889,10 @@ jvmtiHeapFollowRefs_getStackData(J9JVMTIHeapData * iteratorData, J9MM_StackSlotD if (NULL != search.ref) { result = hashTableFind(iteratorData->env->objectTagTable, &search); - /* Figure out the Thread ID */ + /* Retrieve the Thread ID. */ threadID = J9VMJAVALANGTHREAD_TID(iteratorData->currentThread, stackSlotDescriptor->vmThread->threadObject); } else { - /* walking a Continuation, ref to threadObject is lost, set 0 for tag/ID. */ + /* Set 0 for tag/ID since ref to threadObject can be lost while walking a continuation. */ result = NULL; threadID = 0; } diff --git a/runtime/jvmti/jvmtiHeap10.c b/runtime/jvmti/jvmtiHeap10.c index 457be308c8d..dac04d2e781 100644 --- a/runtime/jvmti/jvmtiHeap10.c +++ b/runtime/jvmti/jvmtiHeap10.c @@ -462,14 +462,14 @@ processObjectRef(J9JVMTIObjectIteratorData * data, J9JVMTIObjectTag * entry, jlo static jvmtiIterationControl -processStackRoot(J9JVMTIObjectIteratorData * data, J9JVMTIObjectTag * entry, jlong size, jlong classTag, jvmtiHeapRootKind kind, J9MM_StackSlotDescriptor *stackSlotDescriptor) +processStackRoot(J9JVMTIObjectIteratorData *data, J9JVMTIObjectTag *entry, jlong size, jlong classTag, jvmtiHeapRootKind kind, J9MM_StackSlotDescriptor *stackSlotDescriptor) { jint slot = -1; jint depth = -1; J9Method *ramMethod = NULL; jmethodID method = (jmethodID) -1; - J9JVMTIObjectTag search; - J9JVMTIObjectTag *result; + J9JVMTIObjectTag search = {NULL, 0}; + J9JVMTIObjectTag *result = NULL; if (NULL != stackSlotDescriptor->walkState) { J9StackWalkState *walkState = stackSlotDescriptor->walkState; @@ -477,23 +477,22 @@ processStackRoot(J9JVMTIObjectIteratorData * data, J9JVMTIObjectTag * entry, jlo depth = (jint) walkState->framesWalked; ramMethod = walkState->method; - /* Convert internal slot type to JVMTI type */ + /* Convert internal slot type to JVMTI type. */ switch (walkState->slotType) { - case J9_STACKWALK_SLOT_TYPE_JNI_LOCAL: - kind = JVMTI_HEAP_ROOT_JNI_LOCAL; - break; - case J9_STACKWALK_SLOT_TYPE_METHOD_LOCAL: - kind = JVMTI_HEAP_ROOT_STACK_LOCAL; - break; - default: - kind = JVMTI_HEAP_ROOT_JNI_LOCAL; - ramMethod = NULL; - break; + case J9_STACKWALK_SLOT_TYPE_JNI_LOCAL: + kind = JVMTI_HEAP_ROOT_JNI_LOCAL; + break; + case J9_STACKWALK_SLOT_TYPE_METHOD_LOCAL: + kind = JVMTI_HEAP_ROOT_STACK_LOCAL; + break; + default: + kind = JVMTI_HEAP_ROOT_JNI_LOCAL; + ramMethod = NULL; + break; } /* If there's no method, slot, method and depth are set to -1 by default. */ - if (NULL != ramMethod) { /* Cheating here - should be current thread, but the walk thread will do. */ method = getCurrentMethodID(walkState->walkThread, ramMethod);