Optimize array access macros to reduce using is-contiguous check #20888

LinHu2016 · 2025-01-06T21:46:08Z

Optimize array access macros to use is-contiguous check only in presence
of arraylets

Along with new offheap feature support, Macro Array Element Access
J9JAVAARRAY_EA/J9JAVAARRAY_EA_VM need 1 or 2 more runtime flags check to
handle new cases/new structures. extra runtime condition checks in the
Macro, which is called in hotspot, could cause certain performance
regression in some platforms. especially for standard GC cases, off-heap
would not contribute any benefit at all.

Optimize Macro J9JAVAARRAY_EA/J9JAVAARRAY_EA_VM to avoid runtime check
IsContiguousArray for stamdard GC and balanced GC with off-heap eanable
cases, because there is no discontigouos array for these cases(except o
size array).
1, replace isVirtualLargeObjectHeapEnabled and
isIndexableDataAddrPresent flag with indexableObjectLayout in JavaVM and
J9VmThread.
2, indexableObjectLayout have only 4 states
J9IndexableObjectLayout_NoDataAddr_NoArraylet 0x0 /* StandardGC case /
J9IndexableObjectLayout_NoDataAddr_Arraylet 0x1 / Metronome GC case /
J9IndexableObjectLayout_DataAddr_NoArraylet 0x2 / Balanced GC Offheap
enabled case /
J9IndexableObjectLayout_DataAddr_Arraylet 0x3 / Balanced GC Offheap
disabled case */
3, in Macro J9JAVAARRAY_EA/J9JAVAARRAY_EA_VM, check StandardGC case and
Balanced GC Offheap enabled case first in order to reduce runtime
condition check for standard GC(Balanced GC Offheap enabled case too),
then avoid performance regression(reduce flag numbers in J9VmThread
also could help reduce the regression for some platforms).
4, flag isVirtualLargeObjectHeapEnabled and isIndexableDataAddrPresent
has been removed from J9VmThread
flag isVirtualLargeObjectHeapEnabled has been removed from JavaVM (we
still keep flag isIndexableDataAddrPresent in JavaVM for minimizing the
changes).

Signed-off-by: lhu [email protected]

amicic · 2025-01-08T17:36:04Z

runtime/oti/j9nonbuilder.h

@@ -5647,6 +5655,10 @@ typedef struct J9VMThread {
 #endif /* defined(J9VM_OPT_JFR) */
 } J9VMThread;

+#if defined(J9VM_ENV_DATA64)
+#define J9VMTHREAD_ARRAYLET_ENABLED(vmThread) (J9IndexableObjectLayout_ArrayletMask & (vmThread)->indexableObjectLayout)
+#define J9VMTHREAD_IS_INDEXBLEDATAADDRPRESENT(vmThread) (J9IndexableObjectLayout_DataAddrMask & (vmThread)->indexableObjectLayout)


add extra '_' for easier reading J9VMTHREAD_IS_INDEXBLE_DATAADDR_PRESENT
add 'IS' to J9VMTHREAD_IS_ARRAYLET_ENABLED

amicic · 2025-01-08T17:45:19Z

runtime/oti/j9nonbuilder.h

@@ -6093,7 +6105,8 @@ typedef struct J9JavaVM {
 	UDATA discontiguousIndexableHeaderSize;
 #if defined(J9VM_ENV_DATA64)
 	U_32 isIndexableDataAddrPresent;


since this is being explicitly used by DDR, let's revert the type back to what was before we put recent Offheap changes and what was also the type the last time we released JVM (which is UDATA)

eventually, we should remove it altogether (but that would need some extra DDR changes to handle cores with and without this field)

LinHu2016 · 2025-01-12T18:54:32Z

personal builds
https://hyc-runtimes-jenkins.swg-devops.com/view/OpenJ9%20-%20Personal/job/Pipeline-Build-Test-Personal/25813/
http://vmfarm.rtp.raleigh.ibm.com/build_info.php?build_id=85574

amicic · 2025-01-13T16:39:30Z

runtime/gc_modron_startup/mminit.cpp

-		vm->unsafeIndexableHeaderSize = vm->contiguousIndexableHeaderSize;
-	}
+	/* set default unsafeIndexableHeaderSize */
+	vm->unsafeIndexableHeaderSize = vm->contiguousIndexableHeaderSize;


let's do changes related to setting unsafeIndexableHeaderSize as a separate PR before we proceed with array access macro changes

that looks like something should have been done as the part of original offheap changes (we should not have queried isVirtualLargeObjectHeapEnabled in a relatively common code path). the new way is cleaner (set default, and override it for offheap enabled on its own specific path)

dmitripivkine · 2025-01-14T19:22:12Z

runtime/oti/j9accessbarrier.h

-		? (((vmThread)->isVirtualLargeObjectHeapEnabled) \
-				? J9JAVAARRAYCONTIGUOUS_WITH_DATAADDRESS_VIRTUALLARGEOBJECTHEAPENABLED_EA(vmThread, array, index, elemType) \
-				: J9JAVAARRAYCONTIGUOUS_WITH_DATAADDRESS_VIRTUALLARGEOBJECTHEAPDISABLED_EA(vmThread, array, index, elemType)) \
-		: J9JAVAARRAYCONTIGUOUS_BASE_EA(vmThread, array, index, elemType))


I think we are removing only usage of two DISABLED macros above. These macros can be removed.

amicic · 2025-01-14T19:29:05Z

runtime/oti/j9accessbarrier.h

-	(J9JAVAVM_COMPRESS_OBJECT_REFERENCES(javaVM) \
-		? (0 != ((J9IndexableObjectContiguousCompressed *)(array))->size) \
-		: (0 != ((J9IndexableObjectContiguousFull *)(array))->size))
-
 /* TODO: queries compressed twice - optimize? */
 #define J9JAVAARRAY_EA(vmThread, array, index, elemType) (J9ISCONTIGUOUSARRAY(vmThread, array) ? J9JAVAARRAYCONTIGUOUS_EA(vmThread, array, index, elemType) : J9JAVAARRAYDISCONTIGUOUS_EA(vmThread, array, index, elemType))


as we discussed in person, we could optimized 32 bit similarly, although there are only 2 layouts (with and without arraylet and no dataAddr),

but let's do it in a separate change

amicic · 2025-01-14T19:33:03Z

runtime/gc_realtime/ConfigurationRealtime.cpp

@@ -108,7 +108,9 @@ MM_Heap *
 MM_ConfigurationRealtime::createHeapWithManager(MM_EnvironmentBase *env, uintptr_t heapBytesRequested, MM_HeapRegionManager *regionManager)
 {
 	MM_GCExtensionsBase *extensions = env->getExtensions();
-
+	J9JavaVM *vm = (J9JavaVM *)extensions->getOmrVM()->_language_vm;
+	/* set indexableObjectLayout = J9IndexableObjectLayout_NoDataAddr_Arraylet (metronome GC)*/


the code line is fairly self explanatory, but if you want to add a comment let's try be a bit more creative rather than just repeating the code:

"Let VM know that Metronome has discontiguous indexable object (arraylet layout)"

amicic · 2025-01-14T19:36:03Z

runtime/gc_vlhgc/ConfigurationIncrementalGenerational.cpp

@@ -136,6 +136,9 @@ MM_ConfigurationIncrementalGenerational::createHeapWithManager(MM_EnvironmentBas

 #if defined(J9VM_ENV_DATA64)
 	extensions->indexableObjectModel.setIsDataAddressPresent(true);
+	J9JavaVM *vm = (J9JavaVM *)extensions->getOmrVM()->_language_vm;
+	/* set indexableObjectLayout = J9IndexableObjectLayout_DataAddr_Arraylet (Balanced GC and off-heap disabled) */


again, be a more creative, something like:

"Let VM know that indexable objects in Balanced always have dataAddr, and let's assume initially it has arraylets, which can be later overridden if Offheap is Enabled."

amicic · 2025-01-14T19:37:27Z

runtime/gc_vlhgc/ConfigurationIncrementalGenerational.cpp

-			/* reset vm->isVirtualLargeObjectHeapEnabled and vm->unsafeIndexableHeaderSize for off-heap case */
-			J9JavaVM *vm = (J9JavaVM *)extensions->getOmrVM()->_language_vm;
-			vm->isVirtualLargeObjectHeapEnabled = TRUE;
+			/* set indexableObjectLayout = J9IndexableObjectLayout_DataAddr_NoArraylet (Balanced GC and off-heap enabled) */


"Overriding the original assumption that Balanced has arraylets."

amicic · 2025-01-14T21:16:04Z

Change the title to be more specific:

Optimize array access macros to use is-contiguous check only in presence of arraylets

amicic · 2025-01-15T15:49:23Z

runtime/oti/j9nonbuilder.h

@@ -6347,6 +6357,10 @@ typedef struct J9JavaVM {
 #define J9JAVAVM_OBJECT_HEADER_SIZE(vm) (J9JAVAVM_COMPRESS_OBJECT_REFERENCES(vm) ? sizeof(J9ObjectCompressed) : sizeof(J9ObjectFull))
 #define J9JAVAVM_CONTIGUOUS_INDEXABLE_HEADER_SIZE(vm) ((vm)->contiguousIndexableHeaderSize)
 #define J9JAVAVM_DISCONTIGUOUS_INDEXABLE_HEADER_SIZE(vm) ((vm)->discontiguousIndexableHeaderSize)
+#if defined(J9VM_ENV_DATA64)
+#define J9JAVAVM_ARRAYLET_ENABLED(vm) (J9IndexableObjectLayout_ArrayletMask & (vm)->indexableObjectLayout)
+#define J9JAVAVM_IS_INDEXBLEDATAADDRPRESENT(vm) (J9IndexableObjectLayout_DataAddrMask & (vm)->indexableObjectLayout)


make the name consistent with those that us vmThread

amicic · 2025-01-15T19:55:55Z

runtime/oti/j9nonbuilder.h

@@ -5540,8 +5547,7 @@ typedef struct J9VMThread {
 	UDATA contiguousIndexableHeaderSize;
 	UDATA discontiguousIndexableHeaderSize;
 #if defined(J9VM_ENV_DATA64)
-	UDATA isIndexableDataAddrPresent;
-	U_32 isVirtualLargeObjectHeapEnabled;
+	U_32 indexableObjectLayout; /* can be J9IndexableObjectLayout_NoDataAddr_NoArraylet,J9IndexableObjectLayout_DataAddr_NoArraylet, J9IndexableObjectLayout_NoDataAddr_Arraylet, J9IndexableObjectLayout_DataAddr_Arraylet */


Either this comment should be shorter, some like "valid values are #define J9IndexableObjectLayout_xyz"
or we don't any comment here (as is the case with other fields) but we comment just above J9IndexableObjectLayout #defines that they are used in indexableObjectLayout field.

(same for J9JavaVM)

amicic · 2025-01-15T19:56:51Z

runtime/oti/j9accessbarrier.h

+ * 		if contigious
+ * 			contiguous
+ * 		else
+ * 			discontigous vc


what's the meaning of 'vc'?

it was a typo. removed it

amicic · 2025-01-16T14:47:18Z

runtime/oti/j9nonbuilder.h

@@ -140,6 +140,13 @@
 #define J9ArrayShape32Bit 0x2
 #define J9ArrayShape64Bit 0x3

+/* J9IndexableObjectLayout_xxx are used in indexableObjectLayout fieldo */


typo in 'fieldo' (extra 'o' at the end)
and while you are editing it, add '... field in J9VMThread and J9JavaVM'

Optimize array access macros to use is-contiguous check only in presence of arraylets Along with new offheap feature support, Macro Array Element Access J9JAVAARRAY_EA/J9JAVAARRAY_EA_VM need 1 or 2 more runtime flags check to handle new cases/new structures. extra runtime condition checks in the Macro, which is called in hotspot, could cause certain performance regression in some platforms. especially for standard GC cases, off-heap would not contribute any benefit at all. Optimize Macro J9JAVAARRAY_EA/J9JAVAARRAY_EA_VM to avoid runtime check IsContiguousArray for stamdard GC and balanced GC with off-heap eanable cases, because there is no discontigouos array for these cases(except o size array). 1, replace isVirtualLargeObjectHeapEnabled and isIndexableDataAddrPresent flag with indexableObjectLayout in JavaVM and J9VmThread. 2, indexableObjectLayout have only 4 states J9IndexableObjectLayout_NoDataAddr_NoArraylet 0x0 /* StandardGC case */ J9IndexableObjectLayout_NoDataAddr_Arraylet 0x1 /* Metronome GC case */ J9IndexableObjectLayout_DataAddr_NoArraylet 0x2 /* Balanced GC Offheap enabled case */ J9IndexableObjectLayout_DataAddr_Arraylet 0x3 /* Balanced GC Offheap disabled case */ 3, in Macro J9JAVAARRAY_EA/J9JAVAARRAY_EA_VM, check StandardGC case and Balanced GC Offheap enabled case first in order to reduce runtime condition check for standard GC(Balanced GC Offheap enabled case too), then avoid performance regression(reduce flag numbers in J9VmThread also could help reduce the regression for some platforms). 4, flag isVirtualLargeObjectHeapEnabled and isIndexableDataAddrPresent has been removed from J9VmThread flag isVirtualLargeObjectHeapEnabled has been removed from JavaVM (we still keep flag isIndexableDataAddrPresent in JavaVM for minimizing the changes). Signed-off-by: lhu <[email protected]>

amicic · 2025-01-16T15:35:13Z

jenkins test sanity aix jdk21

keithc-ca · 2025-01-17T13:52:02Z

DDR needs to be updated to adapt to the removal of the field J9JavaVM.isIndexableDataAddrPresent.

amicic · 2025-01-17T14:54:00Z

DDR needs to be updated to adapt to the removal of the field J9JavaVM.isIndexableDataAddrPresent.

isIndexableDataAddrPresent was removed only from J9VMThread (and that one was not referenced by DDR), but not from J9JavaVM.

keithc-ca · 2025-01-17T15:00:11Z

Sorry, you're right, J9JavaVM.isIndexableDataAddrPresent is still present.

LinHu2016 force-pushed the off-heap-optimize4gencon branch 3 times, most recently from 38fe9a6 to b09c9b9 Compare January 8, 2025 16:50

amicic reviewed Jan 8, 2025

View reviewed changes

LinHu2016 force-pushed the off-heap-optimize4gencon branch 11 times, most recently from 87e01b2 to e94cb71 Compare January 12, 2025 18:46

amicic reviewed Jan 13, 2025

View reviewed changes

LinHu2016 force-pushed the off-heap-optimize4gencon branch from e94cb71 to 347dfcb Compare January 14, 2025 15:26

dmitripivkine reviewed Jan 14, 2025

View reviewed changes

amicic reviewed Jan 14, 2025

View reviewed changes

LinHu2016 force-pushed the off-heap-optimize4gencon branch from 347dfcb to 961b71f Compare January 14, 2025 22:31

amicic reviewed Jan 15, 2025

View reviewed changes

LinHu2016 force-pushed the off-heap-optimize4gencon branch from 961b71f to 31701a1 Compare January 15, 2025 18:47

LinHu2016 changed the title ~~Optimize Macro J9JAVAARRAY_EA to reduce offheap changes side affection~~ Optimize array access macros to reduce using is-contiguous check Jan 15, 2025

amicic reviewed Jan 15, 2025

View reviewed changes

LinHu2016 force-pushed the off-heap-optimize4gencon branch from 31701a1 to 36c306a Compare January 15, 2025 22:50

amicic reviewed Jan 16, 2025

View reviewed changes

LinHu2016 force-pushed the off-heap-optimize4gencon branch from 36c306a to cd7ba08 Compare January 16, 2025 15:16

amicic merged commit 5b2c4ec into eclipse-openj9:master Jan 16, 2025
6 checks passed

keithc-ca mentioned this pull request Jan 17, 2025

Fix spelling of contiguous/discontiguous #20966

Merged

amicic added the comp:gc label Mar 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize array access macros to reduce using is-contiguous check #20888

Optimize array access macros to reduce using is-contiguous check #20888

LinHu2016 commented Jan 6, 2025 •

edited

Loading

amicic Jan 8, 2025

amicic Jan 8, 2025

LinHu2016 commented Jan 12, 2025

amicic Jan 13, 2025 •

edited

Loading

dmitripivkine Jan 14, 2025 •

edited

Loading

amicic Jan 14, 2025

amicic Jan 14, 2025

amicic Jan 14, 2025 •

edited

Loading

amicic Jan 14, 2025

amicic commented Jan 14, 2025

amicic Jan 15, 2025

amicic Jan 15, 2025

amicic Jan 15, 2025

LinHu2016 Jan 15, 2025 •

edited

Loading

amicic Jan 16, 2025

amicic commented Jan 16, 2025

keithc-ca commented Jan 17, 2025

amicic commented Jan 17, 2025

keithc-ca commented Jan 17, 2025

Optimize array access macros to reduce using is-contiguous check #20888

Optimize array access macros to reduce using is-contiguous check #20888

Conversation

LinHu2016 commented Jan 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LinHu2016 commented Jan 12, 2025

amicic Jan 13, 2025 • edited Loading

Choose a reason for hiding this comment

dmitripivkine Jan 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amicic Jan 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amicic commented Jan 14, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LinHu2016 Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amicic commented Jan 16, 2025

keithc-ca commented Jan 17, 2025

amicic commented Jan 17, 2025

keithc-ca commented Jan 17, 2025

LinHu2016 commented Jan 6, 2025 •

edited

Loading

amicic Jan 13, 2025 •

edited

Loading

dmitripivkine Jan 14, 2025 •

edited

Loading

amicic Jan 14, 2025 •

edited

Loading

LinHu2016 Jan 15, 2025 •

edited

Loading