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

Avoid double scan stacks in continuation for compact cases #16022

Merged
merged 1 commit into from
Oct 5, 2022
Merged

Avoid double scan stacks in continuation for compact cases #16022

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion runtime/gc/gctable.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ J9MemoryManagerFunctions MemoryManagerFunctions = {
ownableSynchronizerObjectCreated,
continuationObjectCreated,
preMountContinuation,
postDismountContinuation,
postUnmountContinuation,
j9gc_notifyGCOfClassReplacement,
j9gc_get_jit_string_dedup_policy,
j9gc_stringHashFn,
Expand Down
2 changes: 1 addition & 1 deletion runtime/gc_base/gc_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ extern J9_CFUNC UDATA ownableSynchronizerObjectCreated(J9VMThread *vmThread, j9o
extern J9_CFUNC UDATA continuationObjectCreated(J9VMThread *vmThread, j9object_t object);

extern J9_CFUNC void preMountContinuation(J9VMThread *vmThread, j9object_t object);
extern J9_CFUNC void postDismountContinuation(J9VMThread *vmThread, j9object_t object);
extern J9_CFUNC void postUnmountContinuation(J9VMThread *vmThread, j9object_t object);

extern J9_CFUNC void j9gc_notifyGCOfClassReplacement(J9VMThread *vmThread, J9Class *originalClass, J9Class *replacementClass, UDATA isFastHCR);

Expand Down
3 changes: 1 addition & 2 deletions runtime/gc_base/modronapi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,10 @@ void
preMountContinuation(J9VMThread *vmThread, j9object_t object)
{
/* need read barrier to handle concurrent scavenger and gcpolicy:metronome case */

}

void
postDismountContinuation(J9VMThread *vmThread, j9object_t object)
postUnmountContinuation(J9VMThread *vmThread, j9object_t object)
{
/* Conservatively assume that via mutations of stack slots (which are not subject to access barriers),
* all post-write barriers have been triggered on this Continuation object, since it's been mounted. */
Expand Down
2 changes: 1 addition & 1 deletion runtime/gc_base/modronapi.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ UDATA ownableSynchronizerObjectCreated(J9VMThread *vmThread, j9object_t object);

UDATA continuationObjectCreated(J9VMThread *vmThread, j9object_t object);
void preMountContinuation(J9VMThread *vmThread, j9object_t object);
void postDismountContinuation(J9VMThread *vmThread, j9object_t object);
void postUnmountContinuation(J9VMThread *vmThread, j9object_t object);

/**
* Called during class redefinition to notify the GC of replaced classes.In certain cases the GC needs to
Expand Down
6 changes: 5 additions & 1 deletion runtime/gc_glue_java/CompactSchemeFixupObject.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,11 @@ MM_CompactSchemeFixupObject::fixupContinuationObject(MM_EnvironmentStandard *env
fixupMixedObject(objectPtr);
/* fixup Java Stacks in J9VMContinuation */
J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread();
if (VM_VMHelpers::needScanStacksForContinuation(currentThread, objectPtr)) {
/* In sliding compaction we must fix slots exactly once. Since we will fixup stack slots of
* 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, true)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We need a comment here, something like:
In sliding compaction we must fix slots exactly once. Since we will fixup stack slots of mounted Virtual threads later during root fixup, we will skip it during this heap fixup pass (hence passing true(false) for scanOnly(Dis)Mounted parameter)

StackIteratorData4CompactSchemeFixupObject localData;
localData.compactSchemeFixupObject = this;
localData.env = env;
Expand Down
6 changes: 5 additions & 1 deletion runtime/gc_vlhgc/WriteOnceCompactor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,11 @@ MM_WriteOnceCompactor::fixupContinuationObject(MM_EnvironmentVLHGC* env, J9Objec
fixupMixedObject(env, objectPtr, cache);

J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread();
if (VM_VMHelpers::needScanStacksForContinuation(currentThread, objectPtr)) {
/* In sliding compaction we must fix slots exactly once. Since we will fixup stack slots of
* 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, true)) {
StackIteratorData4WriteOnceCompactor localData;
localData.writeOnceCompactor = this;
localData.env = env;
Expand Down
25 changes: 23 additions & 2 deletions runtime/oti/VMHelpers.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2057,13 +2057,34 @@ class VM_VMHelpers
return rc;
}

#if JAVA_SPEC_VERSION >= 19
/**
* Check if the related J9VMContinuation is mounted to carrier thread
* @param[in] continuation the related J9VMContinuation
* @return true if it is mounted.
*/
static VMINLINE bool
needScanStacksForContinuation(J9VMThread *vmThread, j9object_t continuationObject) {
isContinuationMounted(J9VMContinuation *continuation)
{
return (NULL != continuation->carrierThread);
}
#endif /* JAVA_SPEC_VERSION >= 19 */

/**
* Check if we need to scan the java stack for the Continuation Object
* @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 scanOnlyUnmounted = false)
{
bool needScan = false;
#if JAVA_SPEC_VERSION >= 19
jboolean started = J9VMJDKINTERNALVMCONTINUATION_STARTED(vmThread, continuationObject);
J9VMContinuation *continuation = J9VMJDKINTERNALVMCONTINUATION_VMREF(vmThread, continuationObject);
needScan = started && (NULL != continuation);
needScan = started && (NULL != continuation) && (!scanOnlyUnmounted || !isContinuationMounted(continuation));
#endif /* JAVA_SPEC_VERSION >= 19 */
return needScan;
}
Expand Down
3 changes: 2 additions & 1 deletion runtime/oti/j9nonbuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4500,7 +4500,7 @@ typedef struct J9MemoryManagerFunctions {
UDATA ( *ownableSynchronizerObjectCreated)(struct J9VMThread *vmThread, j9object_t object) ;
UDATA ( *continuationObjectCreated)(struct J9VMThread *vmThread, j9object_t object) ;
void ( *preMountContinuation)(struct J9VMThread *vmThread, j9object_t object) ;
void ( *postDismountContinuation)(struct J9VMThread *vmThread, j9object_t object) ;
void ( *postUnmountContinuation)(struct J9VMThread *vmThread, j9object_t object) ;

void ( *j9gc_notifyGCOfClassReplacement)(struct J9VMThread *vmThread, J9Class *originalClass, J9Class *replacementClass, UDATA isFastHCR) ;
I_32 ( *j9gc_get_jit_string_dedup_policy)(struct J9JavaVM *javaVM) ;
Expand Down Expand Up @@ -4998,6 +4998,7 @@ typedef struct J9VMContinuation {
UDATA* j2iFrame;
struct J9JITGPRSpillArea jitGPRs;
struct J9VMEntryLocalStorage* oldEntryLocalStorage;
struct J9VMThread* carrierThread;
} J9VMContinuation;
#endif /* JAVA_SPEC_VERSION >= 19 */

Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/BytecodeInterpreter.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5232,7 +5232,7 @@ class INTERPRETER_CLASS

j9object_t continuationObject = J9VMJAVALANGTHREAD_CONT(_currentThread, _currentThread->carrierThreadObject);
/* Notify GC of Continuation stack swap */
_vm->memoryManagerFunctions->postDismountContinuation(_currentThread, continuationObject);
_vm->memoryManagerFunctions->postUnmountContinuation(_currentThread, continuationObject);

VMStructHasBeenUpdated(REGISTER_ARGS);
restoreInternalNativeStackFrame(REGISTER_ARGS);
Expand Down
3 changes: 2 additions & 1 deletion runtime/vm/ContinuationHelpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ enterContinuation(J9VMThread *currentThread, j9object_t continuationObject)
Assert_VM_Null(currentThread->currentContinuation);

VM_ContinuationHelpers::swapFieldsWithContinuation(currentThread, continuation);
continuation->carrierThread = currentThread;
currentThread->currentContinuation = continuation;

/* Reset counters which determine if the current continuation is pinned. */
Expand Down Expand Up @@ -142,7 +143,7 @@ yieldContinuation(J9VMThread *currentThread)
Assert_VM_notNull(currentThread->currentContinuation);

VM_ContinuationHelpers::swapFieldsWithContinuation(currentThread, continuation);

continuation->carrierThread = NULL;
currentThread->currentContinuation = NULL;

/* We need a full fence here to preserve happens-before relationship on PPC and other weakly
Expand Down