Loom: GC Scan Continuation Object #15603

LinHu2016 · 2022-07-21T21:13:04Z

Identify if the Object is an instance of Continuation, if it is, scan
the Java stacks in related J9vmContinuation.

- add ContinuationLink as a hiddenInstanceField of Continuation
      instance for the global linklist
- openjdk using sub class "java/lang/VirtualThread$VThreadContinuation"
 instead of "jdk/internal/vm/Continuation" for creating instance of
     Continuation.
- new HeapWalkerDelegate to handle special treatment for Continuation
Instance.
- new Scan Type GC_ObjectModel::SCAN_CONTINUATION_OBJECT
- new J9AccClassContinuation bit (0x1000000, bit in classDepthAndFlags)
for identifying Continuation Instance, reset AccClassIsContended bit
(copy from romClass->extraModifiers, but not been used at all).
- scan/special treatment the Java stack related Continuation Instance
	CompactSchemeFixupObject
	CopyForwardScheme
	GlobalCollectorDelegate
	GlobalMarkCardScrbber
	HeapWalkerDelegate
	MarkingDelegate
	MetronomeDelegate
	RealtimeMarkingScheme
	ScavengerDelegate

- ToDo List, below the items might also need some change to match
 continuation, but not directly block the loom basic features.
	HeapIteratorAPI::j9mm_iterate_object_slots()
	MM_ReferenceChainWalker::scanObject()
	MM_CompactDelegate::verifyHeap()
	MM_RealtimeAccessBarrier::validateWriteBarrier()
 	tgcHookReportInterRegionReferenceCounting()
	MM_CopyForwardScheme::verifyObject()
	MM_IncrementalGenerationalGC::verifyMarkMapClosure()
	MM_WriteOnceCompactor::verifyHeap()
	jvmtiHeap.mapEventType()
	DDR
	GCCheck
   - need to handle/avoid mount/unmount continuation during concurrent GCs

#depends on eclipse/omr#6575, #15434, #15678
#fix: #15178

Signed-off-by: Lin Hu linhu@ca.ibm.com

LinHu2016 · 2022-07-27T15:36:42Z

@amicic @dmitripivkine please review the changes, Thanks

dmitripivkine · 2022-07-27T16:23:37Z

runtime/gc_api/HeapIteratorAPI.cpp

+	case GC_ObjectModel::SCAN_CONTINUATION_OBJECT:
+		returnCode = iterateMixedObjectSlots(javaVM, objectPtr, object, flags, func, userData);
+		break;
+#endif /* JAVA_SPEC_VERSION >= 19 */


Do we plan to have extra or different logic here? If not, you can add this case to group above:
....
case GC_ObjectModel::SCAN_CLASSLOADER_OBJECT:
case GC_ObjectModel::SCAN_REFERENCE_MIXED_OBJECT:
#if JAVA_SPEC_VERSION >= 19
case GC_ObjectModel::SCAN_CONTINUATION_OBJECT:
#endif /* JAVA_SPEC_VERSION >= 19 */
....

I leave it separate here, because I am not very clear if we need any different logic for continuation in this case, keep a room to update in future.

dmitripivkine · 2022-07-27T16:27:02Z

General question: Do we need put #if JAVA_SPEC_VERSION >= 19 around most changes in GC? Can we just rely on existence of this case for Java 19 only? If it is possible it would make GC code much cleaner (despite slight grows of code and data for earlier Java versions)

runtime/gc_base/ObjectAccessBarrier.cpp

dmitripivkine · 2022-07-27T16:32:42Z

runtime/gc_base/ReferenceChainWalker.cpp

+	case GC_ObjectModel::SCAN_CONTINUATION_OBJECT:
+		scanMixedObject(objectPtr);
+		break;
+#endif /* JAVA_SPEC_VERSION >= 19 */


Should this case be separate or can it be joined to the group above?

same reason as above for keeping the separated code.

runtime/gc_base/ReferenceChainWalker.hpp

runtime/gc_glue_java/CompactSchemeFixupObject.cpp

LinHu2016 · 2022-08-17T14:31:38Z

updating new vm api walkContinuationStackFrames for scanning java stack in j9vmContinuation.
https://github.com/eclipse-openj9/openj9/pull/15678/files#diff-aa08fd4ba250064714cc602eda8db8bbaae659c38b78e47f24ff00d9a5dd6a44

runtime/gc_vlhgc/MemorySubSpaceTarok.hpp

runtime/gc_base/modronapi.hpp

runtime/vm/createramclass.cpp

jdmpapin · 2022-08-26T20:13:13Z

The JIT changes LGTM, but we may also need to bump MINOR_NUMBER. I'm thinking of a scenario where the client has continuation classes, but the server doesn't know about them, in which case the server could e.g. think it's ok to stack allocate

jdmpapin

Approving, but I'd like @dsouzai to give his opinion about the protocol version

dsouzai

Yeah, the MINOR_NUMBER should be bumped since this includes frontend API changes.

Also there's some formatting issues that remain.

dsouzai · 2022-08-29T17:20:16Z

runtime/compiler/env/J9ClassEnv.cpp

@@ -87,7 +87,8 @@ J9::ClassEnv::isClassSpecialForStackAllocation(TR_OpaqueClassBlock * clazz)
   const UDATA mask = (J9AccClassReferenceWeak |
                       J9AccClassReferenceSoft |
                       J9AccClassFinalizeNeeded |
-                       J9AccClassOwnableSynchronizer);
+                       J9AccClassOwnableSynchronizer |
+					   J9AccClassContinuation );


Indentation is using tabs instead of spaces.

dsouzai · 2022-08-29T17:20:34Z

runtime/compiler/env/VMJ9.cpp

+	   J9Class* j9class = TR::Compiler->cls.convertClassOffsetToClassPtr(clazz);
+	   return ((J9CLASS_FLAGS(j9class) & J9AccClassContinuation) != 0);


Formatting; tabs used instead of spaces.

dsouzai · 2022-08-30T14:34:33Z

@LinHu2016 sorry, in the formatting changes I requested I meant that your code was using tabs, whereas the compiler code uses spaces. As it stands, the formatting is still incorrect in runtime/compiler/env/J9ClassEnv.cpp and runtime/compiler/env/VMJ9.cpp

runtime/compiler/net/CommunicationStream.hpp

amicic · 2022-09-01T16:35:53Z

Jenkins test sanity all jdk11

dmitripivkine

There are two cosmetic changes left:

remove check for Java 19 in MM_GlobalMarkingScheme::scanClassObject() or explain why it required
revert Copyrights date update in MemorySubSpaceTarok.cpp, file has not been changed
Except this looks good to me

amicic · 2022-09-02T15:14:17Z

Jenkins test sanity all jdk11

amicic · 2022-09-02T16:43:42Z

Jenkins compile win32 jdk8

Identify if the Object is an instance of Continuation, if it is, scan the Java stacks in related J9vmContinuation. - add ContinuationLink as a hiddenInstanceField of Continuation instance for the global linklist - openjdk using sub class "java/lang/VirtualThread$VThreadContinuation" instead of "jdk/internal/vm/Continuation" for creating instance of Continuation. - new HeapWalkerDelegate to handle special treatment for Continuation Instance. - new Scan Type GC_ObjectModel::SCAN_CONTINUATION_OBJECT - new J9AccClassContinuation bit (0x1000000, bit in classDepthAndFlags) for identifying Continuation Instance, reset AccClassIsContended bit (copy from romClass->extraModifiers, but not been used at all). - scan/special treatment the Java stack related Continuation Instance CompactSchemeFixupObject CopyForwardScheme GlobalCollectorDelegate GlobalMarkCardScrbber HeapWalkerDelegate MarkingDelegate MetronomeDelegate RealtimeMarkingScheme ScavengerDelegate - ToDo List, below the items might also need some change to match continuation, but not directly block the loom basic features. HeapIteratorAPI::j9mm_iterate_object_slots() MM_ReferenceChainWalker::scanObject() MM_CompactDelegate::verifyHeap() MM_RealtimeAccessBarrier::validateWriteBarrier() tgcHookReportInterRegionReferenceCounting() MM_CopyForwardScheme::verifyObject() MM_IncrementalGenerationalGC::verifyMarkMapClosure() MM_WriteOnceCompactor::verifyHeap() jvmtiHeap.mapEventType() DDR GCCheck Signed-off-by: Lin Hu <linhu@ca.ibm.com>

LinHu2016 · 2022-09-02T17:34:34Z

@amicic Update ScavengerDelegate.cpp, add OMR_GC_CONCURRENT_SCAVENGER precompile option for calling fixupSlot().

amicic · 2022-09-02T17:49:36Z

Jenkins compile win32 jdk8

amicic · 2022-09-02T17:50:19Z

Jenkins compile win jdk11

amicic · 2022-09-02T19:05:17Z

The last commit was minor, fixing a 32bit compile failure. So, I just re-lauched a couple of compile jobs. For sanity testing I'm relying on these that were done just before the last commit, and they were all green: https://openj9-jenkins.osuosl.org/job/PullRequest-OpenJ9/2642/

LinHu2016 force-pushed the GC_SCAN_Continuation_Loom branch 3 times, most recently from dc2a5af to faebda8 Compare July 23, 2022 03:30

This was referenced Jul 25, 2022

Loom: GC collection of Continuations #15611

Merged

Loom: GC Thread scanning #15612

Merged

LinHu2016 force-pushed the GC_SCAN_Continuation_Loom branch from faebda8 to b5fc3a4 Compare July 26, 2022 14:11

dmitripivkine reviewed Jul 27, 2022

View reviewed changes

runtime/gc_base/ObjectAccessBarrier.cpp Outdated Show resolved Hide resolved

dmitripivkine reviewed Jul 27, 2022

View reviewed changes

dmitripivkine reviewed Jul 28, 2022

View reviewed changes

runtime/gc_base/ReferenceChainWalker.hpp Outdated Show resolved Hide resolved

dmitripivkine reviewed Jul 28, 2022

View reviewed changes

runtime/gc_glue_java/CompactSchemeFixupObject.cpp Outdated Show resolved Hide resolved

LinHu2016 force-pushed the GC_SCAN_Continuation_Loom branch 7 times, most recently from 3fb509a to fe627a9 Compare August 3, 2022 20:15

dmitripivkine added comp:gc project:loom Used to track Project Loom related work labels Aug 4, 2022

dmitripivkine requested review from dmitripivkine and amicic August 4, 2022 13:04

LinHu2016 force-pushed the GC_SCAN_Continuation_Loom branch 2 times, most recently from d873db9 to c9b5e0c Compare August 4, 2022 16:52

LinHu2016 force-pushed the GC_SCAN_Continuation_Loom branch 2 times, most recently from f20dd73 to 07da4a5 Compare August 17, 2022 14:22

LinHu2016 force-pushed the GC_SCAN_Continuation_Loom branch from 07da4a5 to 63192ac Compare August 17, 2022 21:36

dmitripivkine reviewed Aug 24, 2022

View reviewed changes

runtime/gc_vlhgc/MemorySubSpaceTarok.hpp Outdated Show resolved Hide resolved

LinHu2016 force-pushed the GC_SCAN_Continuation_Loom branch from eaf733a to c1d7e88 Compare August 24, 2022 23:43

dmitripivkine reviewed Aug 25, 2022

View reviewed changes

runtime/gc_base/modronapi.hpp Outdated Show resolved Hide resolved

tajila reviewed Aug 25, 2022

View reviewed changes

runtime/vm/createramclass.cpp Outdated Show resolved Hide resolved

LinHu2016 force-pushed the GC_SCAN_Continuation_Loom branch 2 times, most recently from 9a25d9e to 2af6bac Compare August 26, 2022 01:08

tajila approved these changes Aug 26, 2022

View reviewed changes

dmitripivkine added comp:vm comp:jit labels Aug 26, 2022

jdmpapin approved these changes Aug 26, 2022

View reviewed changes

dsouzai requested changes Aug 29, 2022

View reviewed changes

LinHu2016 force-pushed the GC_SCAN_Continuation_Loom branch 2 times, most recently from 121cb31 to 9182142 Compare August 30, 2022 14:33

dsouzai reviewed Aug 30, 2022

View reviewed changes

runtime/compiler/net/CommunicationStream.hpp Outdated Show resolved Hide resolved

LinHu2016 force-pushed the GC_SCAN_Continuation_Loom branch from 9182142 to 1fe5a82 Compare August 30, 2022 14:43

dsouzai approved these changes Aug 30, 2022

View reviewed changes

dmitripivkine approved these changes Sep 1, 2022

View reviewed changes

LinHu2016 force-pushed the GC_SCAN_Continuation_Loom branch from 1fe5a82 to 5502e96 Compare September 1, 2022 18:07

LinHu2016 force-pushed the GC_SCAN_Continuation_Loom branch from 5502e96 to 69c8e12 Compare September 2, 2022 17:29

amicic merged commit 00f03c2 into eclipse-openj9:master Sep 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Loom: GC Scan Continuation Object #15603

Loom: GC Scan Continuation Object #15603

LinHu2016 commented Jul 21, 2022 •

edited

LinHu2016 commented Jul 27, 2022

dmitripivkine Jul 27, 2022

LinHu2016 Jul 27, 2022

dmitripivkine commented Jul 27, 2022 •

edited

dmitripivkine Jul 27, 2022

LinHu2016 Jul 27, 2022

LinHu2016 commented Aug 17, 2022

jdmpapin commented Aug 26, 2022

jdmpapin left a comment

dsouzai left a comment

dsouzai Aug 29, 2022

dsouzai Aug 29, 2022

dsouzai commented Aug 30, 2022

amicic commented Sep 1, 2022

dmitripivkine left a comment

amicic commented Sep 2, 2022

amicic commented Sep 2, 2022

LinHu2016 commented Sep 2, 2022

amicic commented Sep 2, 2022

amicic commented Sep 2, 2022

amicic commented Sep 2, 2022 •

edited

		J9Class* j9class = TR::Compiler->cls.convertClassOffsetToClassPtr(clazz);
		return ((J9CLASS_FLAGS(j9class) & J9AccClassContinuation) != 0);

Loom: GC Scan Continuation Object #15603

Loom: GC Scan Continuation Object #15603

Conversation

LinHu2016 commented Jul 21, 2022 • edited

LinHu2016 commented Jul 27, 2022

dmitripivkine Jul 27, 2022

Choose a reason for hiding this comment

LinHu2016 Jul 27, 2022

Choose a reason for hiding this comment

dmitripivkine commented Jul 27, 2022 • edited

dmitripivkine Jul 27, 2022

Choose a reason for hiding this comment

LinHu2016 Jul 27, 2022

Choose a reason for hiding this comment

LinHu2016 commented Aug 17, 2022

jdmpapin commented Aug 26, 2022

jdmpapin left a comment

Choose a reason for hiding this comment

dsouzai left a comment

Choose a reason for hiding this comment

dsouzai Aug 29, 2022

Choose a reason for hiding this comment

dsouzai Aug 29, 2022

Choose a reason for hiding this comment

dsouzai commented Aug 30, 2022

amicic commented Sep 1, 2022

dmitripivkine left a comment

Choose a reason for hiding this comment

amicic commented Sep 2, 2022

amicic commented Sep 2, 2022

LinHu2016 commented Sep 2, 2022

amicic commented Sep 2, 2022

amicic commented Sep 2, 2022

amicic commented Sep 2, 2022 • edited

LinHu2016 commented Jul 21, 2022 •

edited

dmitripivkine commented Jul 27, 2022 •

edited

amicic commented Sep 2, 2022 •

edited