Skip to content

Detect cold CallTarget invalidation and reset its profile; Limit number of recompilations within a time period #11610

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Detect cold CallTarget invalidation and reset its profile; Limit number of recompilations within a time period #11610

wants to merge 10 commits into from

Conversation

JohnTortugo
Copy link
Contributor

Closes: #11045

Please review this patch that modifies Truffle to:

  1. Reset a CallTarget's profile when the nmethod associated with it was invalidated by HotSpot because the Code Cache's method flushing heuristics deemed the nmethod to be cold.

  2. Limit the number of compilations that a CallTarget can have within a time period. This doesn't change the current behavior, instead it just makes the constraint more flexible. I added a new parameter, MaximumCompilationsWindow, that the user can set to specify a time window within which the number of compilations will be limited (currently the window is whole duration of application execution).

Tests:

  • OSX AArch64 with LabsJDK "tip" using "mx gate"
  • Linux x65 with LabsJDK "tip" using "mx gate"
  • GitHub Actions
  • We have been running an internal system with similar changes for months and we didn't notice any regression.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Jul 12, 2025
@JohnTortugo JohnTortugo changed the title Detect cold method invalidation & reprofile. Limit on number of compilations Detect cold CallTarget invalidation and reset its profile; Limit number of recompilations within a time period Jul 12, 2025
@JohnTortugo
Copy link
Contributor Author

Can someone please take a look? /cc @chumer @dougxc ?

@dougxc
Copy link
Member

dougxc commented Jul 22, 2025

Looks reasonable to me but a more detailed review should be done by @tzezula or @chumer.

chumer
chumer requested changes Jul 22, 2025
View reviewed changes
@JohnTortugo
Copy link
Contributor Author

JohnTortugo commented Aug 16, 2025 •
edited
Loading

@chumer, I did a refactoring of the code following your suggestions. Basically, I changed two things:

  • I dropped the changes related to resetting number of compilations based on time. The approach now is to reset successfullRecompilationCount when the method was invalidated because it was cold.
  • I reverted the changes in HotspotOptimizedCallTarget to detect that the method was invalidated. I implemented the idea that you suggested: verifying the invalidation reason of the call target when it's about to be recompiled.

I tested the latest changes using the approach described in the issue ticket and everything worked fine. mx unittest and mx gate also didn't raise any issue.

@JohnTortugo JohnTortugo requested a review from chumer August 20, 2025 20:07
@JohnTortugo
Copy link
Contributor Author

Gentle ping. @chumer - Please, take a look when you can.

chumer
chumer requested changes Sep 4, 2025
View reviewed changes
...com.oracle.truffle.runtime/src/com/oracle/truffle/runtime/hotspot/HotSpotTruffleRuntime.java Outdated
HotSpotOptimizedCallTarget hsCallTarget = (HotSpotOptimizedCallTarget) callTarget;
if (hsCallTarget.getInvalidationReason() == this.coldMethodInvalidationReason) {
hsCallTarget.resetCompilationProfile();
hsCallTarget.resetInstalledCode();
Copy link
Member

Choose a reason for hiding this comment

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

How do we know that this is not racy with compilations installing code at the same time?
Or is this something we would accept?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My view here is that no two threads will run this method concurrently, because the call only happens while compilationTask in the CT is != null. Also, my understanding is that even if for some reason the system ends up invalidating a "good" compilation because of a race condition: 1) for Truffle it shouldn't matter because it will at the same time be installing new code and 2) HotSpot will just drop the previous compilation from the CC because it will eventually be cold. Please, let me know if this doesn't make sense.

I moved this method to HotSpotOptimizedCallTarget::prepareForCompilation as you suggested in another comment.

@JohnTortugo
Copy link
Contributor Author

@chumer - I believe I addressed all your comments. Please, take a look again when you can.

@chumer chumer requested a review from jchalou September 16, 2025 12:37
@chumer
Copy link
Member

chumer commented Sep 16, 2025 •
edited
Loading

@jchalou please give this another pair of eyes. If its fine feel free to integrate.

@chumer chumer self-requested a review September 16, 2025 12:38
@chumer
Copy link
Member

chumer commented Sep 16, 2025

@JohnTortugo any chance we could have a smoke test for this behavior? That is reasonably complex? Would be great to be notified if this breaks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chumer chumer chumer approved these changes

@jchalou jchalou Awaiting requested review from jchalou

Assignees

@chumer chumer

Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Truffle OptimizedCallTarget profile counters do not decay
3 participants
@JohnTortugo @dougxc @chumer