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

Update Object.wait to use Blocker.begin/end for Java 19+ #16324

Merged
merged 1 commit into from
Nov 22, 2022

Conversation

fengxue-IS
Copy link
Contributor

@fengxue-IS fengxue-IS commented Nov 14, 2022

Fix: #16198

Signed-off-by: Jack Lu Jack.S.Lu@ibm.com

@fengxue-IS fengxue-IS marked this pull request as ready for review November 15, 2022 01:37
@fengxue-IS
Copy link
Contributor Author

Personal build https://hyc-runtimes-jenkins.swg-devops.com/view/OpenJ9%20-%20Personal/job/Pipeline-Build-Test-Personal/14803/

@tajila can you please take a look
@0xdaryl can you review the JIT changes please, I only found 1 instance where Object.wait is hardcoded in JIT

@gacholio
Copy link
Contributor

I think it might be best to add waitImpl to the final method list and leave wait in there - these are methods that do not require vTable entries (so we would save one slot in every class). Please give it a try and see if it causes any problems.

@fengxue-IS
Copy link
Contributor Author

I think it might be best to add waitImpl to the final method list and leave wait in there - these are methods that do not require vTable entries (so we would save one slot in every class). Please give it a try and see if it causes any problems.

I've modified final.c to also include waitImpl and increased the OBJECT_FINAL_COUNT.
Did I miss any place that have a separate list?

@gacholio
Copy link
Contributor

Sorry, I misread the diff - you have done exactly what I was suggesting.

@gacholio
Copy link
Contributor

Is this the approach hotspot took? I'm concerned that people know too much about wait being native and this may cause issues with instrumentation agents.

@fengxue-IS
Copy link
Contributor Author

fengxue-IS commented Nov 17, 2022

RI have a similar approach to add Object.wait0
Even before this change, RI uses native wait(long milli) as the base where OpenJ9 uses native wait(long milli, int nano) as base.

@gacholio
Copy link
Contributor

RI uses native wait(long milli) as the base

Does this mean they don't support nanos (ignore it and forward to the above native) or are there multiple natives?

@fengxue-IS
Copy link
Contributor Author

RI uses native wait(long milli) as the base

Does this mean they don't support nanos (ignore it and forward to the above native) or are there multiple natives?

They round nanos to millis

@gacholio
Copy link
Contributor

They round nanos to millis

Perhaps we should do the same (not in this PR).

Given the existing differences between the implementations, I believe instrumentation agents are unlikely to be affected by this change.

Signed-off-by: Jack Lu <Jack.S.Lu@ibm.com>
@fengxue-IS
Copy link
Contributor Author

Addressed review comments, local testing passed on xlinux, personal build in progress: https://hyc-runtimes-jenkins.swg-devops.com/view/OpenJ9%20-%20Personal/job/Pipeline-Build-Test-Personal/14854/

@gacholio
Copy link
Contributor

jenkins test sanity zlinux jdk8,jdk19

@gacholio
Copy link
Contributor

jenkins compile win jdk19

@gacholio gacholio merged commit defdf9a into eclipse-openj9:master Nov 22, 2022
@tajila tajila added backport:candidate Possible candidate for a backport to a `0.x.y+1` release jdk19 labels Nov 29, 2022
@pshipton pshipton removed the backport:candidate Possible candidate for a backport to a `0.x.y+1` release label Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jdk19 OpenJDK java/lang/Thread/virtual/ActiviateSpareCarrier.java timeout
5 participants