From a9e4bc92eea7a43be03c998c29db3bbf8ad1bbc6 Mon Sep 17 00:00:00 2001 From: Lin Hu Date: Mon, 12 Dec 2022 09:29:24 -0500 Subject: [PATCH] Avoid to scan the finished Continuation Object There is no need to scan any continuation Objects, which are marked finished, plus there could be potential race condition between continuation concurrent scan and continuation free up for finished Continuation Object. Signed-off-by: Lin Hu --- runtime/gc_base/GCExtensions.cpp | 28 ++++++++++++++++ runtime/gc_base/GCExtensions.hpp | 12 ++++++- .../gc_glue_java/CompactSchemeFixupObject.cpp | 4 +-- runtime/gc_glue_java/HeapWalkerDelegate.cpp | 5 +-- runtime/gc_glue_java/MarkingDelegate.cpp | 4 +-- runtime/gc_glue_java/MetronomeDelegate.cpp | 4 +-- runtime/gc_glue_java/ScavengerDelegate.cpp | 4 +-- runtime/gc_vlhgc/CopyForwardScheme.cpp | 4 +-- runtime/gc_vlhgc/GlobalMarkCardScrubber.cpp | 4 +-- runtime/gc_vlhgc/GlobalMarkingScheme.cpp | 4 +-- runtime/gc_vlhgc/WriteOnceCompactor.cpp | 4 +-- runtime/oti/VMHelpers.hpp | 33 +------------------ 12 files changed, 59 insertions(+), 51 deletions(-) diff --git a/runtime/gc_base/GCExtensions.cpp b/runtime/gc_base/GCExtensions.cpp index 6fce57a216a..75a67039e57 100644 --- a/runtime/gc_base/GCExtensions.cpp +++ b/runtime/gc_base/GCExtensions.cpp @@ -43,6 +43,7 @@ #include "ReferenceChainWalkerMarkMap.hpp" #include "SublistPool.hpp" #include "Wildcard.hpp" +#include "VMHelpers.hpp" MM_GCExtensions * MM_GCExtensions::newInstance(MM_EnvironmentBase *env) @@ -311,3 +312,30 @@ MM_GCExtensions::releaseNativesForContinuationObject(MM_EnvironmentBase* env, j9 } #endif /* JAVA_SPEC_VERSION >= 19 */ } + +bool +MM_GCExtensions::needScanStacksForContinuationObject(J9VMThread *vmThread, j9object_t objectPtr) +{ + bool needScan = false; +#if JAVA_SPEC_VERSION >= 19 + jboolean started = J9VMJDKINTERNALVMCONTINUATION_STARTED(vmThread, objectPtr); + jboolean finished = J9VMJDKINTERNALVMCONTINUATION_FINISHED(vmThread, objectPtr); + J9VMContinuation *continuation = J9VMJDKINTERNALVMCONTINUATION_VMREF(vmThread, objectPtr); + /** + * We don't scan mounted continuations: + * + * for concurrent GCs, since stack is actively changing. Instead, we scan them during preMount or during root scanning if already mounted at cycle start or during postUnmount (might be indirectly via card cleaning) or during final STW (via root re-scan) if still mounted at cycle end + * for sliding compacts to avoid double slot fixups + * + * 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 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); + } +#endif /* JAVA_SPEC_VERSION >= 19 */ + return needScan; +} diff --git a/runtime/gc_base/GCExtensions.hpp b/runtime/gc_base/GCExtensions.hpp index 7201b167fa2..354f8e631f7 100644 --- a/runtime/gc_base/GCExtensions.hpp +++ b/runtime/gc_base/GCExtensions.hpp @@ -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 @@ -299,6 +299,16 @@ class MM_GCExtensions : public MM_GCExtensionsBase { void releaseNativesForContinuationObject(MM_EnvironmentBase* env, j9object_t objectPtr); + /** + * 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 + * @return true if we need to scan the java stack + */ + static bool needScanStacksForContinuationObject(J9VMThread *vmThread, j9object_t objectPtr); + /** * Create a GCExtensions object */ diff --git a/runtime/gc_glue_java/CompactSchemeFixupObject.cpp b/runtime/gc_glue_java/CompactSchemeFixupObject.cpp index a89fd3165f8..442a25733fc 100644 --- a/runtime/gc_glue_java/CompactSchemeFixupObject.cpp +++ b/runtime/gc_glue_java/CompactSchemeFixupObject.cpp @@ -1,5 +1,5 @@ /******************************************************************************* - * 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 @@ -75,7 +75,7 @@ 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 (VM_VMHelpers::needScanStacksForContinuation(currentThread, objectPtr)) { + if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) { StackIteratorData4CompactSchemeFixupObject localData; localData.compactSchemeFixupObject = this; localData.env = env; diff --git a/runtime/gc_glue_java/HeapWalkerDelegate.cpp b/runtime/gc_glue_java/HeapWalkerDelegate.cpp index e19490963aa..3d9ad2eaf81 100644 --- a/runtime/gc_glue_java/HeapWalkerDelegate.cpp +++ b/runtime/gc_glue_java/HeapWalkerDelegate.cpp @@ -1,6 +1,6 @@ /******************************************************************************* - * Copyright (c) 2022, 2022 IBM Corp. and others + * Copyright (c) 2022, 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 @@ -25,6 +25,7 @@ #include "j9.h" #include "EnvironmentBase.hpp" +#include "GCExtensions.hpp" #include "ObjectModel.hpp" #include "VMHelpers.hpp" #include "VMThreadStackSlotIterator.hpp" @@ -59,7 +60,7 @@ MM_HeapWalkerDelegate::doContinuationNativeSlots(MM_EnvironmentBase *env, omrobj { J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread(); - if (VM_VMHelpers::needScanStacksForContinuation(currentThread, objectPtr)) { + if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) { StackIteratorData4HeapWalker localData; localData.heapWalker = _heapWalker; localData.env = env; diff --git a/runtime/gc_glue_java/MarkingDelegate.cpp b/runtime/gc_glue_java/MarkingDelegate.cpp index e5dee69c2fd..9478c53c3fa 100644 --- a/runtime/gc_glue_java/MarkingDelegate.cpp +++ b/runtime/gc_glue_java/MarkingDelegate.cpp @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2017, 2022 IBM Corp. and others + * Copyright (c) 2017, 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 @@ -261,7 +261,7 @@ void MM_MarkingDelegate::scanContinuationNativeSlots(MM_EnvironmentBase *env, omrobjectptr_t objectPtr) { J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread(); - if (VM_VMHelpers::needScanStacksForContinuation(currentThread, objectPtr)) { + if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) { StackIteratorData4MarkingDelegate localData; localData.markingDelegate = this; localData.env = env; diff --git a/runtime/gc_glue_java/MetronomeDelegate.cpp b/runtime/gc_glue_java/MetronomeDelegate.cpp index 29b5a1950a3..f57b40790e3 100644 --- a/runtime/gc_glue_java/MetronomeDelegate.cpp +++ b/runtime/gc_glue_java/MetronomeDelegate.cpp @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2019, 2022 IBM Corp. and others + * Copyright (c) 2019, 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 @@ -1647,7 +1647,7 @@ void MM_MetronomeDelegate::scanContinuationNativeSlots(MM_EnvironmentRealtime *env, J9Object *objectPtr) { J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread(); - if (VM_VMHelpers::needScanStacksForContinuation(currentThread, objectPtr)) { + if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) { StackIteratorData4RealtimeMarkingScheme localData; localData.realtimeMarkingScheme = _markingScheme; localData.env = env; diff --git a/runtime/gc_glue_java/ScavengerDelegate.cpp b/runtime/gc_glue_java/ScavengerDelegate.cpp index 2833eac57c1..a0fb9a35d53 100644 --- a/runtime/gc_glue_java/ScavengerDelegate.cpp +++ b/runtime/gc_glue_java/ScavengerDelegate.cpp @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2019, 2022 IBM Corp. and others + * Copyright (c) 2019, 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 @@ -345,7 +345,7 @@ MM_ScavengerDelegate::scanContinuationNativeSlots(MM_EnvironmentStandard *env, o bool shouldRemember = false; J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread(); - if (VM_VMHelpers::needScanStacksForContinuation(currentThread, objectPtr)) { + if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) { StackIteratorData4Scavenge localData; localData.scavengerDelegate = this; localData.env = env; diff --git a/runtime/gc_vlhgc/CopyForwardScheme.cpp b/runtime/gc_vlhgc/CopyForwardScheme.cpp index 74e1f69fc5c..760a27c6b23 100644 --- a/runtime/gc_vlhgc/CopyForwardScheme.cpp +++ b/runtime/gc_vlhgc/CopyForwardScheme.cpp @@ -1,5 +1,5 @@ /******************************************************************************* - * 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 @@ -2322,7 +2322,7 @@ MMINLINE void MM_CopyForwardScheme::scanContinuationNativeSlots(MM_EnvironmentVLHGC *env, MM_AllocationContextTarok *reservingContext, J9Object *objectPtr, ScanReason reason) { J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread(); - if (VM_VMHelpers::needScanStacksForContinuation(currentThread, objectPtr)) { + if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) { StackIteratorData4CopyForward localData; localData.copyForwardScheme = this; localData.env = env; diff --git a/runtime/gc_vlhgc/GlobalMarkCardScrubber.cpp b/runtime/gc_vlhgc/GlobalMarkCardScrubber.cpp index 3bcf7937d6b..f30d67aaf52 100644 --- a/runtime/gc_vlhgc/GlobalMarkCardScrubber.cpp +++ b/runtime/gc_vlhgc/GlobalMarkCardScrubber.cpp @@ -1,5 +1,5 @@ /******************************************************************************* - * 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 @@ -192,7 +192,7 @@ bool MM_GlobalMarkCardScrubber::scrubContinuationNativeSlots(MM_EnvironmentVLHGC { bool doScrub = true; J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread(); - if (VM_VMHelpers::needScanStacksForContinuation(currentThread, objectPtr)) { + if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) { StackIteratorData4GlobalMarkCardScrubber localData; localData.globalMarkCardScrubber = this; localData.env = env; diff --git a/runtime/gc_vlhgc/GlobalMarkingScheme.cpp b/runtime/gc_vlhgc/GlobalMarkingScheme.cpp index 9e757262b04..532c258e825 100644 --- a/runtime/gc_vlhgc/GlobalMarkingScheme.cpp +++ b/runtime/gc_vlhgc/GlobalMarkingScheme.cpp @@ -1,5 +1,5 @@ /******************************************************************************* - * 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 @@ -792,7 +792,7 @@ void MM_GlobalMarkingScheme::scanContinuationNativeSlots(MM_EnvironmentVLHGC *env, J9Object *objectPtr, ScanReason reason) { J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread(); - if (VM_VMHelpers::needScanStacksForContinuation(currentThread, objectPtr)) { + if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) { StackIteratorData4GlobalMarkingScheme localData; localData.globalMarkingScheme = this; localData.env = env; diff --git a/runtime/gc_vlhgc/WriteOnceCompactor.cpp b/runtime/gc_vlhgc/WriteOnceCompactor.cpp index 9e5eb9ff51c..2dd22f27654 100644 --- a/runtime/gc_vlhgc/WriteOnceCompactor.cpp +++ b/runtime/gc_vlhgc/WriteOnceCompactor.cpp @@ -1,5 +1,5 @@ /******************************************************************************* - * 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 @@ -1240,7 +1240,7 @@ 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 (VM_VMHelpers::needScanStacksForContinuation(currentThread, objectPtr)) { + if (MM_GCExtensions::needScanStacksForContinuationObject(currentThread, objectPtr)) { StackIteratorData4WriteOnceCompactor localData; localData.writeOnceCompactor = this; localData.env = env; diff --git a/runtime/oti/VMHelpers.hpp b/runtime/oti/VMHelpers.hpp index ead29009979..8009cf4ae39 100644 --- a/runtime/oti/VMHelpers.hpp +++ b/runtime/oti/VMHelpers.hpp @@ -1,5 +1,5 @@ /******************************************************************************* - * 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 @@ -2126,37 +2126,6 @@ class VM_VMHelpers return rc; } - /** - * 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] scanOnlyUnmounted if it is true, only scan unmounted continuation object, default is false - * @return true if we need to scan the java stack - */ - static VMINLINE bool - needScanStacksForContinuation(J9VMThread *vmThread, j9object_t continuationObject) - { - bool needScan = false; -#if JAVA_SPEC_VERSION >= 19 - jboolean started = J9VMJDKINTERNALVMCONTINUATION_STARTED(vmThread, continuationObject); - J9VMContinuation *continuation = J9VMJDKINTERNALVMCONTINUATION_VMREF(vmThread, continuationObject); - /** - * We don't scan mounted continuations: - * - * for concurrent GCs, since stack is actively changing. Instead, we scan them during preMount or during root scanning if already mounted at cycle start or during postUnmount (might be indirectly via card cleaning) or during final STW (via root re-scan) if still mounted at cycle end - * for sliding compacts to avoid double slot fixups - * - * 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. - */ - needScan = started && (NULL != continuation) && (!isContinuationMountedOrConcurrentlyScanned(continuation)); -#endif /* JAVA_SPEC_VERSION >= 19 */ - return needScan; - } - static VMINLINE UDATA setVMState(J9VMThread *currentThread, UDATA newState) {