Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix missing concurrent continuation scanning issue #16614

Merged
merged 1 commit into from
Feb 4, 2023
Merged

Fix missing concurrent continuation scanning issue #16614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 5 additions & 3 deletions runtime/gc_base/GCExtensions.cpp
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sometimes you have an empty line between the setting concurrent flag and this line, and sometimes not
let's be consistent, and have an empty line (here and check other spots, too)

}
}

Expand Down
7 changes: 4 additions & 3 deletions runtime/gc_glue_java/ScavengerDelegate.cpp
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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