Minimize continuation Object size in continuation list #17554
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LinHu2016
force-pushed
the
GC_Loom_update_19
branch
from
0e23151
to
2f90a1e
Compare
amicic
reviewed
Jul 5, 2023
runtime/gc_base/modronapi.cpp
Outdated
| { | ||
| Assert_MM_true(NULL != object); | ||
| MM_EnvironmentBase *env = MM_EnvironmentBase::getEnvironment(vmThread->omrVMThread); | ||
| if (MM_GCExtensions::onFirstMount == MM_GCExtensions::getExtensions(env)->timingAddContinuationInList) { |
There was a problem hiding this comment.
let's rename this enum to onStarted to be consistent with the rest of related APIs/states
amicic
reviewed
Jul 5, 2023
| @@ -325,7 +327,7 @@ MM_MarkingSchemeRootClearer::scanContinuationObjects(MM_EnvironmentBase *env) | |||
| while (NULL != object) { | |||
| gcEnv->_markJavaStats._continuationCandidates += 1; | |||
| omrobjectptr_t next = _extensions->accessBarrier->getContinuationLink(object); | |||
| if (_markingScheme->isMarked(object)) { | |||
| if (_markingScheme->isMarked(object) && !VM_VMHelpers::isContinuationFinished((J9VMThread *)env->getLanguageVMThread(), object)) { | |||
There was a problem hiding this comment.
why did you create this helper - just for being concise?
we already used VM_ContinuationHelpers::isFinished() directly in MM_GCExtensions::releaseNativesForContinuationObject
There was a problem hiding this comment.
more importantly, why did you create it in VM_VMHelpers (we recently moved out bunch of Continuation specific ones to VM_ContinuationHelpers)?
There was a problem hiding this comment.
the code was started before we merged the changes(move out Continuation specific ones to VM_ContinuationHelpers)
LinHu2016
force-pushed
the
GC_Loom_update_19
branch
from
2f90a1e
to
974748e
Compare
amicic
reviewed
Jul 5, 2023
| @@ -3543,7 +3544,7 @@ MM_CopyForwardScheme::scanContinuationObjects(MM_EnvironmentVLHGC *env) | |||
| */ | |||
| MM_ForwardedHeader forwardedHeader(pointer, _extensions->compressObjectReferences()); | |||
| J9Object* forwardedPtr = forwardedHeader.getForwardedObject(); | |||
There was a problem hiding this comment.
Move '*' next to variable (as opposed to next to type). Fix it for this method or any other method you that you've touched.
amicic
reviewed
Jul 5, 2023
| } else { | ||
| omrobjectptr_t forwardedPtr = forwardedHeader.getForwardedObject(); | ||
| Assert_GC_true_with_message(env, NULL != forwardedPtr, "Continuation object %p should be forwarded\n", object); |
There was a problem hiding this comment.
this assert might be too late - we might have already dereferenced forwardedPtr as part of isContinuationFinished, perhaps move it just after getForwardedObject?
LinHu2016
force-pushed
the
GC_Loom_update_19
branch
2 times, most recently
from
a8ee2a5
to
7ab2479
Compare
amicic
reviewed
Jul 5, 2023
runtime/oti/VMHelpers.hpp
Outdated
| @@ -43,6 +43,9 @@ | |||
| #include "ute.h" | |||
| #include "AtomicSupport.hpp" | |||
| #include "ObjectAllocationAPI.hpp" | |||
| #if JAVA_SPEC_VERSION >= 19 | |||
| #include "ContinuationHelpers.hpp" | |||
| #endif /* JAVA_SPEC_VERSION >= 19 */ | |||
LinHu2016
force-pushed
the
GC_Loom_update_19
branch
5 times, most recently
from
058f7c4
to
eeb7708
Compare
|
jenkins test sanity aix,win jdk20 |
Currently GC maintain the global continuation lists contain all of "live" continuation Object for releasing native resource(exceptional case), jvmti retrieving all continuation and JIT code cache reclaim processing. keep less items in the list would reduce time to refresh/iterate the list. - new Java Option -XXgc:AddContinuationInListOnStarted (default, would not add unstarted continuation in the list) and -XXgc:AddContinuationInListOnCreated. - remove not only "dead" but also finished continuation during refreshing list. Signed-off-by: Lin Hu <linhu@ca.ibm.com>
LinHu2016
force-pushed
the
GC_Loom_update_19
branch
from
eeb7708
to
bbdd543
Compare
|
jenkins test sanity aix,win jdk20 |
amicic
approved these changes
Jul 10, 2023
babsingh
added a commit
to babsingh/openj9
that referenced
this pull request
Jul 17, 2023
Related: eclipse-openj9#17554 Currently, the default mode is “onStarted”. This adds the continuation to the list only when the VirtualThread (or the Continuation) starts. The “onCreated” mode adds to the list as soon as the VirtualThread (or the Continuation) object is created. This mode is needed if JVMTI is used. So, the default is changed to “onCreated” until we can dynamically determine that JVMTI won’t be used. With “onStarted” as default, the following failures started to occur last week: - eclipse-openj9#17784 - eclipse-openj9#17791 (intermittent) - eclipse-openj9#17782 (intermittent) Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently GC maintain the global continuation lists contain all of
"live" continuation Object for releasing native resource(exceptional
case), jvmti retrieving all continuation and JIT code cache reclaim
processing.
keep less items in the list would reduce time to refresh/iterate the list.