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

Retry failed profiled very-hot compilations at hot instead of warm #21053

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

mpirvu
Copy link
Contributor

@mpirvu mpirvu commented Feb 1, 2025

Before this change, a profiled-very-hot compilation that fails due to excessiveComplexity would not be retried, relying on the existing compiled body. If the existing compiled body is at warm (or even cold) opt level, then the performance could be suboptimal.

With this commit profiled-very-hot compilations that fail will be retried at the hot optimization level, but without any profiling. We avoid profiling compilations because they are more complex than non-profiled ones and the likelihood of a failure for a profiled-hot compilation is relatively high when we know that a profiled-very-hot compilation for the same method already failed.

@mpirvu mpirvu added the comp:jit label Feb 1, 2025
@mpirvu mpirvu requested a review from dsouzai as a code owner February 1, 2025 16:53
Before this change, a profiled-very-hot compilation that fails
due to excessiveComplexity would not be retried, relying on the
existing compiled body. If the existing compiled body is at
warm (or even cold) opt level, then the performance could be
suboptimal.

With this commit profiled-very-hot compilations that fail will be
retried at the hot optimization level, but without any profiling.
We avoid profiling compilations because they are more complex than
non-profiled ones and the likelihood of a failure for a
profiled-hot compilation is relatively high when we know that a
profiled-very-hot compilation for the same method already failed.

Signed-off-by: Marius Pirvu <[email protected]>
@mpirvu
Copy link
Contributor Author

mpirvu commented Feb 1, 2025

jenkins test sanity all jdk8

@vijaysun-omr vijaysun-omr self-assigned this Feb 1, 2025
@vijaysun-omr
Copy link
Contributor

I would appreciate a quick look from @dsouzai too

Copy link
Contributor

@dsouzai dsouzai left a comment

Choose a reason for hiding this comment

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

lgtm

@mpirvu
Copy link
Contributor Author

mpirvu commented Feb 3, 2025

jenkins test sanity xmac jdk8

@mpirvu
Copy link
Contributor Author

mpirvu commented Feb 3, 2025

I restarted the xmac build since it failed due to not finding a machine to run.

@mpirvu
Copy link
Contributor Author

mpirvu commented Feb 3, 2025

All checks have passed. This PR is ready to be merged.

@JamesKingdon
Copy link
Contributor

I ran this change against a testcase that demonstrated the problem and confirmed that it restored the expected performance.

Many thanks!

@vijaysun-omr vijaysun-omr merged commit 6ea0111 into eclipse-openj9:master Feb 3, 2025
27 checks passed
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.

4 participants