Skip to content

Commit

Permalink
Fix missing concurrent continuation scanning issue
Browse files Browse the repository at this point in the history
The issue is caused by concurrent scavenger scanning and concurrent
marking scanning overlap for the same continuation Object, during
concurrent continuation scanning, the current synchronization control
would block the continuation mounting and ignore another concurrent
scanning, but the concurrent scavenger scanning and concurrent marking
are irrelevant, ignore another could cause missing scanning and the
related "live" object is recycled.

- updated J9VMContinuation->state, use two low bits for recording
 concurrentScanning(bit0 for concurrentScanningLocal and bit1 for
 concurrentScanningGlobal) instead of only one low bit.

- pass flag isConcurrentGC and flag isGlobalGC for
GC_VMThreadStackSlotIterator::scanSlots().

- handle J9_GC_CONTINUATION_STATE_CONCURRENT_SCAN_LOCAL and
J9_GC_CONTINUATION_STATE_CONCURRENT_SCAN_GLOBAL independently

- only both J9_GC_CONTINUATION_STATE_CONCURRENT_SCAN_LOCAL and
J9_GC_CONTINUATION_STATE_CONCURRENT_SCAN_GLOBAL bits has been cleared
we can notify blocked the continuation mounting thread.

Signed-off-by: Lin Hu <linhu@ca.ibm.com>
  • Loading branch information
LinHu2016 committed Feb 3, 2023
1 parent 056acd0 commit 13ebc10
Show file tree
Hide file tree
Showing 17 changed files with 133 additions and 65 deletions.
8 changes: 5 additions & 3 deletions runtime/gc_base/GCExtensions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ MM_GCExtensions::releaseNativesForContinuationObject(MM_EnvironmentBase* env, j9
}

bool
MM_GCExtensions::needScanStacksForContinuationObject(J9VMThread *vmThread, j9object_t objectPtr)
MM_GCExtensions::needScanStacksForContinuationObject(J9VMThread *vmThread, j9object_t objectPtr, bool isGlobalGC)
{
bool needScan = false;
#if JAVA_SPEC_VERSION >= 19
Expand All @@ -335,12 +335,14 @@ MM_GCExtensions::needScanStacksForContinuationObject(J9VMThread *vmThread, j9obj
*
* For fully STW GCs, there is no harm to scan them, but it's a waste of time since they are scanned during root scanning already.
*
* We don't scan currently scanned either - one scan is enough.
* We don't scan currently scanned for the same collector either - one scan is enough for the same collector, but there could be concurrent scavenger(local collector) and concurrent marking(global collector) overlapping,
* they are irrelevant and both are concurrent, we handle them independently and separately, they are not blocked or ignored each other.
*
* we don't scan the continuation object before started and after finished - java stack does not exist.
*/
if (started && !finished) {
Assert_MM_true(NULL != continuation);
needScan = !VM_VMHelpers::isContinuationMountedOrConcurrentlyScanned(continuation);
needScan = !VM_VMHelpers::isContinuationMountedOrConcurrentlyScanned(continuation, isGlobalGC);
}
#endif /* JAVA_SPEC_VERSION >= 19 */
return needScan;
Expand Down
4 changes: 3 additions & 1 deletion runtime/gc_base/GCExtensions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -303,11 +303,13 @@ class MM_GCExtensions : public MM_GCExtensionsBase {
* Check if we need to scan the java stack for the Continuation Object
* Used during main scan phase of GC (object graph traversal) or heap object iteration (in sliding compact).
* Not meant to be used during root scanning (neither strong roots nor weak roots)!
*
* @param[in] vmThread the current J9VMThread
* @param[in] continuationObject the continuation object
* @param[in] isGlobalGC
* @return true if we need to scan the java stack
*/
static bool needScanStacksForContinuationObject(J9VMThread *vmThread, j9object_t objectPtr);
static bool needScanStacksForContinuationObject(J9VMThread *vmThread, j9object_t objectPtr, bool isGlobalGC);

/**
* Create a GCExtensions object
Expand Down
6 changes: 4 additions & 2 deletions runtime/gc_glue_java/CompactSchemeFixupObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,15 @@ MM_CompactSchemeFixupObject::fixupContinuationNativeSlots(MM_EnvironmentStandard
* mounted Virtual threads later during root fixup, we will skip it during this heap fixup pass
* (hence passing true for scanOnlyUnmounted parameter).
*/
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) {
const bool isGlobalGC = true;
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr, isGlobalGC)) {
StackIteratorData4CompactSchemeFixupObject localData;
localData.compactSchemeFixupObject = this;
localData.env = env;
localData.fromObject = objectPtr;
const bool isConcurrentGC = false;

GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForCompactScheme, false, false);
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForCompactScheme, false, false, isConcurrentGC, isGlobalGC);
}
}

Expand Down
7 changes: 5 additions & 2 deletions runtime/gc_glue_java/HeapWalkerDelegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,17 @@ MM_HeapWalkerDelegate::doContinuationNativeSlots(MM_EnvironmentBase *env, omrobj
{
J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread();

if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) {
const bool isGlobalGC = true;
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr, isGlobalGC)) {
StackIteratorData4HeapWalker localData;
localData.heapWalker = _heapWalker;
localData.env = env;
localData.fromObject = objectPtr;
localData.function = function;
localData.userData = userData;
/* so far there is no case we need ClassWalk for heapwalker, so we set stackFrameClassWalkNeeded = false */
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForHeapWalker, false, false);
const bool isConcurrentGC = false;

GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForHeapWalker, false, false, isConcurrentGC, isGlobalGC);
}
}
9 changes: 5 additions & 4 deletions runtime/gc_glue_java/MarkingDelegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,8 @@ void
MM_MarkingDelegate::scanContinuationNativeSlots(MM_EnvironmentBase *env, omrobjectptr_t objectPtr)
{
J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread();
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) {
const bool isGlobalGC = true;
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr, isGlobalGC)) {
StackIteratorData4MarkingDelegate localData;
localData.markingDelegate = this;
localData.env = env;
Expand All @@ -271,10 +272,10 @@ MM_MarkingDelegate::scanContinuationNativeSlots(MM_EnvironmentBase *env, omrobje
#if defined(J9VM_GC_DYNAMIC_CLASS_UNLOADING)
stackFrameClassWalkNeeded = isDynamicClassUnloadingEnabled();
#endif /* J9VM_GC_DYNAMIC_CLASS_UNLOADING */

/* In STW GC there are no racing carrier threads doing mount and no need for the synchronization. */
bool syncWithContinuationMounting = J9_ARE_ANY_BITS_SET(currentThread->privateFlags, J9_PRIVATE_FLAGS_CONCURRENT_MARK_ACTIVE);
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForMarkingDelegate, stackFrameClassWalkNeeded, false, syncWithContinuationMounting);
bool isConcurrentGC = J9_ARE_ANY_BITS_SET(currentThread->privateFlags, J9_PRIVATE_FLAGS_CONCURRENT_MARK_ACTIVE);

GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForMarkingDelegate, stackFrameClassWalkNeeded, false, isConcurrentGC, isGlobalGC);
}
}

Expand Down
8 changes: 4 additions & 4 deletions runtime/gc_glue_java/MetronomeDelegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1647,7 +1647,8 @@ void
MM_MetronomeDelegate::scanContinuationNativeSlots(MM_EnvironmentRealtime *env, J9Object *objectPtr)
{
J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread();
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) {
const bool isGlobalGC = true;
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr, isGlobalGC)) {
StackIteratorData4RealtimeMarkingScheme localData;
localData.realtimeMarkingScheme = _markingScheme;
localData.env = env;
Expand All @@ -1657,11 +1658,10 @@ MM_MetronomeDelegate::scanContinuationNativeSlots(MM_EnvironmentRealtime *env, J
#if defined(J9VM_GC_DYNAMIC_CLASS_UNLOADING)
stackFrameClassWalkNeeded = isDynamicClassUnloadingEnabled();
#endif /* J9VM_GC_DYNAMIC_CLASS_UNLOADING */

/* In STW GC there are no racing carrier threads doing mount and no need for the synchronization. */
bool syncWithContinuationMounting = _realtimeGC->isCollectorConcurrentTracing();
bool isConcurrentGC = _realtimeGC->isCollectorConcurrentTracing();

GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForRealtimeGC, stackFrameClassWalkNeeded, false, syncWithContinuationMounting);
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForRealtimeGC, stackFrameClassWalkNeeded, false, isConcurrentGC, isGlobalGC);
}
}

Expand Down
7 changes: 4 additions & 3 deletions runtime/gc_glue_java/ScavengerDelegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -345,16 +345,17 @@ MM_ScavengerDelegate::scanContinuationNativeSlots(MM_EnvironmentStandard *env, o
bool shouldRemember = false;

J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread();
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) {
const bool isGlobalGC = false;
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr, isGlobalGC)) {
StackIteratorData4Scavenge localData;
localData.scavengerDelegate = this;
localData.env = env;
localData.reason = reason;
localData.shouldRemember = &shouldRemember;
/* In STW GC there are no racing carrier threads doing mount and no need for the synchronization. */
bool syncWithContinuationMounting = _extensions->isConcurrentScavengerInProgress();
bool isConcurrentGC = _extensions->isConcurrentScavengerInProgress();

GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForScavenge, false, false, syncWithContinuationMounting);
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForScavenge, false, false, isConcurrentGC, isGlobalGC);
}
return shouldRemember;
}
Expand Down
7 changes: 4 additions & 3 deletions runtime/gc_structs/VMThreadStackSlotIterator.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

/*******************************************************************************
* Copyright (c) 1991, 2022 IBM Corp. and others
* Copyright (c) 1991, 2023 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
Expand Down Expand Up @@ -137,13 +137,14 @@ GC_VMThreadStackSlotIterator::scanSlots(
J9MODRON_OSLOTITERATOR *oSlotIterator,
bool includeStackFrameClassReferences,
bool trackVisibleFrameDepth,
bool syncWithContinuationMounting
bool isConcurrentGC,
bool isGlobalGC
)
{
J9StackWalkState stackWalkState;

initializeStackWalkState(&stackWalkState, vmThread, userData, oSlotIterator, includeStackFrameClassReferences, trackVisibleFrameDepth);
VM_VMHelpers::walkContinuationStackFramesWrapper(vmThread, continuationObjectPtr, &stackWalkState, syncWithContinuationMounting);
VM_VMHelpers::walkContinuationStackFramesWrapper(vmThread, continuationObjectPtr, &stackWalkState, isConcurrentGC, isGlobalGC);
}

#if JAVA_SPEC_VERSION >= 19
Expand Down
5 changes: 3 additions & 2 deletions runtime/gc_structs/VMThreadStackSlotIterator.hpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

/*******************************************************************************
* Copyright (c) 1991, 2022 IBM Corp. and others
* Copyright (c) 1991, 2023 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
Expand Down Expand Up @@ -67,7 +67,8 @@ class GC_VMThreadStackSlotIterator
J9MODRON_OSLOTITERATOR *oSlotIterator,
bool includeStackFrameClassReferences,
bool trackVisibleFrameDepth,
bool syncWithContinuationMounting = false);
bool isConcurrentGC,
bool isGlobalGC);

#if JAVA_SPEC_VERSION >= 19
static void scanSlots(
Expand Down
7 changes: 5 additions & 2 deletions runtime/gc_vlhgc/CopyForwardScheme.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2322,7 +2322,8 @@ MMINLINE void
MM_CopyForwardScheme::scanContinuationNativeSlots(MM_EnvironmentVLHGC *env, MM_AllocationContextTarok *reservingContext, J9Object *objectPtr, ScanReason reason)
{
J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread();
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) {
const bool isGlobalGC = false;
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr, isGlobalGC)) {
StackIteratorData4CopyForward localData;
localData.copyForwardScheme = this;
localData.env = env;
Expand All @@ -2332,7 +2333,9 @@ MM_CopyForwardScheme::scanContinuationNativeSlots(MM_EnvironmentVLHGC *env, MM_A
#if defined(J9VM_GC_DYNAMIC_CLASS_UNLOADING)
stackFrameClassWalkNeeded = isDynamicClassUnloadingEnabled();
#endif /* J9VM_GC_DYNAMIC_CLASS_UNLOADING */
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForCopyForwardScheme, stackFrameClassWalkNeeded, false);
const bool isConcurrentGC = false;

GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForCopyForwardScheme, stackFrameClassWalkNeeded, false, isConcurrentGC, isGlobalGC);
}
}

Expand Down
6 changes: 4 additions & 2 deletions runtime/gc_vlhgc/GlobalMarkCardScrubber.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,14 +192,16 @@ bool MM_GlobalMarkCardScrubber::scrubContinuationNativeSlots(MM_EnvironmentVLHGC
{
bool doScrub = true;
J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread();
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) {
const bool isGlobalGC = true;
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr, isGlobalGC)) {
StackIteratorData4GlobalMarkCardScrubber localData;
localData.globalMarkCardScrubber = this;
localData.env = env;
localData.doScrub = &doScrub;
localData.fromObject = objectPtr;
const bool isConcurrentGC = false;

GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForGlobalMarkCardScrubber, false, false);
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForGlobalMarkCardScrubber, false, false, isConcurrentGC, isGlobalGC);
}
return doScrub;
}
Expand Down
8 changes: 4 additions & 4 deletions runtime/gc_vlhgc/GlobalMarkingScheme.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -792,7 +792,8 @@ void
MM_GlobalMarkingScheme::scanContinuationNativeSlots(MM_EnvironmentVLHGC *env, J9Object *objectPtr, ScanReason reason)
{
J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread();
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) {
const bool isGlobalGC = true;
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr, isGlobalGC)) {
StackIteratorData4GlobalMarkingScheme localData;
localData.globalMarkingScheme = this;
localData.env = env;
Expand All @@ -801,11 +802,10 @@ MM_GlobalMarkingScheme::scanContinuationNativeSlots(MM_EnvironmentVLHGC *env, J9
#if defined(J9VM_GC_DYNAMIC_CLASS_UNLOADING)
stackFrameClassWalkNeeded = isDynamicClassUnloadingEnabled();
#endif /* J9VM_GC_DYNAMIC_CLASS_UNLOADING */

/* In STW GC there are no racing carrier threads doing mount and no need for the synchronization. */
bool syncWithContinuationMounting = (MM_VLHGCIncrementStats::mark_concurrent == static_cast<MM_CycleStateVLHGC*>(env->_cycleState)->_vlhgcIncrementStats._globalMarkIncrementType);
bool isConcurrentGC = (MM_VLHGCIncrementStats::mark_concurrent == static_cast<MM_CycleStateVLHGC*>(env->_cycleState)->_vlhgcIncrementStats._globalMarkIncrementType);

GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForGlobalMarkingScheme, stackFrameClassWalkNeeded, false, syncWithContinuationMounting);
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForGlobalMarkingScheme, stackFrameClassWalkNeeded, false, isConcurrentGC, isGlobalGC);
}
}

Expand Down
6 changes: 4 additions & 2 deletions runtime/gc_vlhgc/WriteOnceCompactor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1240,13 +1240,15 @@ MM_WriteOnceCompactor::fixupContinuationNativeSlots(MM_EnvironmentVLHGC* env, J9
* mounted Virtual threads later during root fixup, we will skip it during this heap fixup pass
* (hence passing true for scanOnlyUnmounted parameter).
*/
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) {
const bool isGlobalGC = MM_CycleState::CT_GLOBAL_GARBAGE_COLLECTION == env->_cycleState->_collectionType;
if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr, isGlobalGC)) {
StackIteratorData4WriteOnceCompactor localData;
localData.writeOnceCompactor = this;
localData.env = env;
localData.fromObject = objectPtr;
const bool isConcurrentGC = false;

GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForWriteOnceCompactor, false, false);
GC_VMThreadStackSlotIterator::scanSlots(currentThread, objectPtr, (void *)&localData, stackSlotIteratorForWriteOnceCompactor, false, false, isConcurrentGC, isGlobalGC);
}
}

Expand Down
Loading

0 comments on commit 13ebc10

Please sign in to comment.