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.

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

Re-introduce double-mapping for non off-heap case #21107

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LinHu2016
Copy link
Contributor

Along with new off-heap feature, double-mapping has been removed (off-heap can cover all of double-mapping cases with better performance), but in case enabling off-heap could not be set as default, re-introduce double-mapping as alternative backup.

1, double mapping is still part of array Discontiguous layout
2, no share code between off-heap and double-mapping

@LinHu2016 LinHu2016 force-pushed the off-heap_doublemapping branch from 39cbb96 to 8eb45cb Compare February 11, 2025 18:42
@@ -281,12 +284,47 @@ MM_VLHGCAccessBarrier::jniGetPrimitiveArrayCritical(J9VMThread* vmThread, jarray
*isCopy = JNI_FALSE;
}

if (alwaysCopyInCritical || !indexableObjectModel->isInlineContiguousArraylet(arrayObject)) {
if (alwaysCopyInCritical) {
Copy link
Contributor

@amicic amicic Feb 11, 2025

Choose a reason for hiding this comment

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

To make eventual removal of double mapping easier we could do this (just thinking loud, don't do it yet)

#if defined(J9VM_GC_ENABLE_DOUBLE_MAP)
	if (alwaysCopyInCritical) {
#else /* defined(J9VM_GC_ENABLE_DOUBLE_MAP) */
	if (alwaysCopyInCritical || !indexableObjectModel->isInlineContiguousArraylet(arrayObject)) {
#endif /* defined(J9VM_GC_ENABLE_DOUBLE_MAP) */

/* alwaysCopyInCritical or discontiguous (including 0 size array) */
copyArrayCritical(vmThread, &data, arrayObject, isCopy);
} else if (indexableObjectModel->isVirtualLargeObjectHeapEnabled() && !indexableObjectModel->isDataAdjacentToHeader(arrayObject)) {
/* off heap enabled and not adjacent */
data = (void *)indexableObjectModel->getDataPointerForContiguous(arrayObject);
} else if (!indexableObjectModel->isInlineContiguousArraylet(arrayObject)) {
/* an array having discontiguous extents is another reason to force the critical section to be a copy */
#if defined(J9VM_GC_ENABLE_DOUBLE_MAP)
Copy link
Contributor

@amicic amicic Feb 11, 2025

Choose a reason for hiding this comment

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

in the light of the previous comment, then this ifdef could be just above

else if (!indexableObjectModel->isInlineContiguousArraylet(arrayObject)) {

end its matching endif should be just after the final

{
	copyArrayCritical(vmThread, &data, arrayObject, isCopy);
}

@LinHu2016 LinHu2016 force-pushed the off-heap_doublemapping branch from 8eb45cb to 88dcd12 Compare February 13, 2025 15:24
Along with new off-heap feature, double-mapping has been removed
(off-heap can cover all of double-mapping cases with better
performance), but in case enabling off-heap could not be set as
default, re-introduce double-mapping as alternative backup.

1, double mapping is still part of array Discontiguous layout
2, no share code between off-heap and double-mapping

Signed-off-by: lhu <[email protected]>
@LinHu2016 LinHu2016 force-pushed the off-heap_doublemapping branch from 88dcd12 to 3d3991a Compare February 25, 2025 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants