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

Disable safe point OSR if sampled object allocate hook is used #17314

Merged
merged 1 commit into from
May 2, 2023

Conversation

gacholio
Copy link
Contributor

@gacholio gacholio commented May 1, 2023

Use of the can_generate_sampled_object_alloc_events JVMTI capability must disable safe point OSR because the event would be sent when not at an OSR safe point, leading to assertions or hangs.

Fixes: #16480

@gacholio gacholio changed the title Disable OSR safe point if sampled object allocate hook is used Disable safe point OSR if sampled object allocate hook is used May 1, 2023
Use of the can_generate_sampled_object_alloc_events JVMTI capability
must disable safe point OSR because the event would be sent when not at
an OSR safe point, leading to assertions or hangs.

Fixes: eclipse-openj9#16480

Signed-off-by: Graham Chapman <[email protected]>
@gacholio gacholio marked this pull request as ready for review May 1, 2023 18:00
@gacholio gacholio requested a review from tajila May 1, 2023 18:00
@tajila
Copy link
Contributor

tajila commented May 1, 2023

FYI @vijaysun-omr @0xdaryl

@tajila
Copy link
Contributor

tajila commented May 1, 2023

jenkins test sanity win jdk8

@tajila
Copy link
Contributor

tajila commented May 1, 2023

jenkins test sanity alinux64 jdk19

@vijaysun-omr
Copy link
Contributor

vijaysun-omr commented May 1, 2023

What does "safe point OSR" mean exactly ? From the JIT perspective we are either in "voluntary OSR" mode (e.g. when HCR is enabled, i.e. our current default) or "involuntary OSR" mode (e.g. used when debug is enabled). Are we saying that this change will put us into involuntary OSR mode (where previously we would have been in voluntary OSR mode) if the relevant allocate hook mentioned earlier is used, or does it mean something else ?

@gacholio
Copy link
Contributor Author

gacholio commented May 1, 2023

The safe point optimization means we can not OSR at an object allocation point - it has nothing to do with voluntary or not. I gather it was a savings on OSR code / metadata.

@vijaysun-omr
Copy link
Contributor

Having fewer program points where OSR is allowed is indeed a savings in code/metadata size and also better in terms of JIT code quality (leads to better throughput). I think I agree that having allocations be (additional) OSR points probably won't result in much performance degradation and so can be done if we need to for that JVMTI capability. I hope that the JIT is set up to check for changes in what is or isn't an OSR point, i.e. that there won't be any functional concerns @hzongaro

@gacholio
Copy link
Contributor Author

gacholio commented May 1, 2023

There's a command line option to disable this already, but we don't run any testing with it, so it's possible that errors might creep into that path.

@gacholio
Copy link
Contributor Author

gacholio commented May 1, 2023

Also, this determination is made before any code generation and will not change throughout the run.

@gacholio
Copy link
Contributor Author

gacholio commented May 2, 2023

Can this be merged?

@tajila tajila merged commit 275bb5a into eclipse-openj9:master May 2, 2023
@gacholio gacholio deleted the sampled branch May 2, 2023 19:06
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.

Assertion failed currentThread->publicFlags
4 participants