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

Adjust arguments to Unsafe array operations while offheap is enabled (FIXED) #20090

Merged
merged 7 commits into from
Aug 30, 2024

Conversation

midronij
Copy link
Contributor

@midronij midronij commented Aug 30, 2024

Updated version of #17969 that addresses #19947.

When Unsafe.get()/put() is called on an array and offheap changes
are enabled, adjust arguments so that dataAddr is passed in as
base address of object

Signed-off-by: midronij <[email protected]>
When Unsafe.compareAndSwap() is called on an array and offheap changes
are enabled, adjust arguments so that dataAddr is passed in as
base address of object

Signed-off-by: midronij <[email protected]>
When the type of the object being passed in to Unsafe.get()/put() is
known at compile time, rather than generate runtime type checks, skip
to determining and generating the appropriate access block. There are
two cases in which we can do this:

1.) If the object is known to be an array, only generate the
    arrayDirectAccessBlock and directAccessBlock (in case NULLCHK
    fails).
2.) If the object is known to be non-array, non-java/lang/Class,
    and non-java/lang/Object, only generate the directAccessBlock.

Signed-off-by: midronij <[email protected]>
For Unsafe.compareAndSwap(), do not generate array check IL or adjusted
array access block if object type is known at compile time.

Signed-off-by: midronij <[email protected]>
When offheap is enabled and the type of the object being operated on
is unknown, there are three possible cases that need to be accounted
for regarding the type of the object being operated on for
Unsafe.get()/put():

1.) array object
2.) java/lang/Class object
3.) none of the above

Since case (3) is the most likely, we want the block corresponding to
this case (directAccessBlock) to be on the fallthrough path.

Signed-off-by: midronij <[email protected]>
@midronij midronij force-pushed the unsafe-debug-testing branch from d9ad8a0 to 4117d7c Compare August 30, 2024 13:55
Add comments and update existing documentation/explanations to reflect
changes made to inlining of Unsafe.get()/put() and compareAndSwap() to
accommodate cases where offheap allocation is enabled.

Signed-off-by: midronij <[email protected]>
A crash was occuring in Unsafe.get/put() case (6) when trying to modify
the CFG to include the newly generated directAccessBlock. To avoid this,
we can simply append the directAccessTreeTop onto the previous block
(beforeCallBlock), which elimiates the need to update the CFG, since no
new blocks are added. Additionally, without this commit, these two blocks
(in addition to the one after the directAccessBlock, joinBlock), would
get merged during block simplication anyways: so this fix eliminates the
need to perform that optimization later down the line.

Signed-off-by: midronij <[email protected]>
@midronij midronij force-pushed the unsafe-debug-testing branch from 4117d7c to f205ea8 Compare August 30, 2024 13:57
@midronij
Copy link
Contributor Author

midronij commented Aug 30, 2024

@zl-wang the fix itself is only in this commit: f205ea8. The others are exactly the same as the original PR.

@midronij midronij changed the title Adjust arguments to Unsafe array operations while offheap is enabled - FIXED Adjust arguments to Unsafe array operations while offheap is enabled (FIXED) Aug 30, 2024
@zl-wang
Copy link
Contributor

zl-wang commented Aug 30, 2024

jenkins test sanity all jdk21

@zl-wang
Copy link
Contributor

zl-wang commented Aug 30, 2024

ignoring pending status due to windows machine issue ... merging

@zl-wang zl-wang merged commit 65af3b4 into eclipse-openj9:master Aug 30, 2024
14 of 16 checks passed
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