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

Support Accelerating ArraysSupport.vectorizedMismatch for OffHeap #21097

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

Conversation

luke-li-2003
Copy link
Contributor

Support the acceleration of vectorizedMisMatch for OffHeap implementation by adding a block of code to calculate the address of the arrays, while accounting for null values.

@luke-li-2003
Copy link
Contributor Author

The code mimics transformUnsafeCopyMemorytoArrayCopyForOffHeap which does something similar. My testing shows the code is working for arrays, but I'm not sure how to test it for non-arrays.

@luke-li-2003
Copy link
Contributor Author

The code probably needs reformatting, since I'm not sure whether I should move some code to J9TransformUtil.cpp

@luke-li-2003
Copy link
Contributor Author

Issue: #16717

@luke-li-2003
Copy link
Contributor Author

FYI @zl-wang

@luke-li-2003 luke-li-2003 force-pushed the OffHeapVectorizedMismatch branch from a98a3d7 to d4ec9e4 Compare February 10, 2025 22:54
Support the acceleration of vectorizedMisMatch for OffHeap implementation
by adding a block of code to calculate the address of the arrays, while
accounting for null values.

Signed-off-by: Luke Li <[email protected]>
@luke-li-2003 luke-li-2003 force-pushed the OffHeapVectorizedMismatch branch from d4ec9e4 to 304b76d Compare February 10, 2025 22:56
@@ -467,8 +467,169 @@ void J9::RecognizedCallTransformer::process_jdk_internal_util_ArraysSupport_vect
TR::Node::create(node, TR::lxor, 2, mask, TR::Node::lconst(node, -1)));

TR::Node* mismatchByteIndex = TR::Node::create(node, TR::arraycmplen, 3);
// TODO: replace the following aladd's with generateDataAddrLoadTrees when off-heap memory changes come in
// See OpenJ9 issue #16717 https://github.com/eclipse-openj9/openj9/issues/16717
#if defined(J9VM_GC_SPARSE_HEAP_ALLOCATION)
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to go through this complicated transformation for offheap? i.e. can we simplify it somewhat? i guessed we need to answer a couple questions before it is cleared up:

  1. if a & b are non-NULL, could they not be array objects? i.e. can we assume they are arrays?
  2. should we treat aOffset & bOffset in the same manner as in Unsafe.get/put()? I think so from language intention perspective.
    when both answers are yes, the transformation for offheap should simplify to (for a as an example):
    (a == NULL)? aOffset : (a.DataAddr + aOffset)

@Spencer-Comin please chime in for your opinion ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. As outlined in Disable vectorizedMismatch transformation for OffHeap #20008 (comment), the jdk does not assert them to be arrays. I double-checked and that was indeed the case, so I was working from that assumption.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah ... it sounds similar to Unsafe.copyMemory() then. @rmnattas can share with you much more details.

Copy link
Contributor

Choose a reason for hiding this comment

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

When vectorizedMismatch is called for array objects the offset includes the array header size (i.e. the offset is from the beginning of the object, not from the address of the first index), so to compensate the correct transformation when a is an offheap array would be a.DataAddr + aOffset - arrayHeaderSize

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI @Spencer-Comin : no need. in offheap mode, ArrayBaseOffset is defined to be zero already.

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.

3 participants