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

Infrastructure for EDO profiling optimization #16799

Merged
merged 3 commits into from
Mar 2, 2023

Conversation

mpirvu
Copy link
Contributor

@mpirvu mpirvu commented Feb 28, 2023

Previous implementation of EDO (Exception Directed Optimization) required 3 compilations to take advantage of the optimization:

  • first compilation would insert a recompilation counter decrement in the catch block
  • second compilation would profile the frequency of the catch block
  • third compilation would make inliner more aggressive on the throw path

We want to take advantage of EDO with just two compilations, by combining the first two steps into one.
This means that every method will now do catch block profiling. This should not have too much overhead because we only insert one increment operation in the catch block and the overhead of an exception is far greater than this extra instruction.

This PR implements the infrastructure required for the mechanism above.

  • It tracks method bodies that have EDO recompilation snippets
  • It records the reason for recompilation in case the recompilation was caused by EDO
  • It adds a _catchBlockCounter field in TR_PersistentMethodInfo that will be used for profiling

The PR also removes the code for the ThresholdCompilationStrategy which has been unused for years now.
Because this cleanup commit removed a counter from TR_PersistentMethodInfo, there is no footprint impact from this PR as a whole.

Added `HasEdoSnippet` flag in `TR_PersistentJittedBodyInfo` which is set
when the codegen generates an EDO recompilation snippet (such snippet
decrements `TR_PersistentJittedBodyInfo._counter` whenever a catch
block is entered).
Added new reason for recompilation in `TR_PersistentMethodInfo` called
`RecompDueToEdo` which is set when a recompilation is triggered from
the snippet mentioned above.
Verbose log will show "E" (from EDO) as the reason for recompilation
when the recompilation was triggered by EDO.

Signed-off-by: Marius Pirvu <mpirvu@ca.ibm.com>
Signed-off-by: Marius Pirvu <mpirvu@ca.ibm.com>
This counter is supposed to be incremented by jitted code
every time a catch block is entered. The value of the
counter should be inspected by the Inliner to determine
whether it should inline more aggressively on the throw
path in order to transform a throw into a goto operation.

Signed-off-by: Marius Pirvu <mpirvu@ca.ibm.com>
@mpirvu
Copy link
Contributor Author

mpirvu commented Feb 28, 2023

@ymanton could you please review/merge this relatively simple PR? Thanks

Copy link
Member

@ymanton ymanton left a comment

Choose a reason for hiding this comment

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

LGTM. It was not obvious to me how _catchBlockCounter was being used until I realized that you are not yet using it in these patches.

@mpirvu
Copy link
Contributor Author

mpirvu commented Mar 1, 2023

I realized that you are not yet using it in these patches.

Yes, I plan to deliver a subsequent omr PR with some other infra and then another openj9 PR with the code that exploits the infra created.

@ymanton
Copy link
Member

ymanton commented Mar 1, 2023

Jenkins test sanity.functional all jdk11

@mpirvu
Copy link
Contributor Author

mpirvu commented Mar 1, 2023

AIX has this infra failure:

11:46:56  Executing command: /bin/sh -c git log --pretty=format:%s -1
11:46:56  Failed fetching commit message from git directory: /home/jenkins/workspace/Build_JDK11_ppc64_aix_Personal/.git
11:46:56  With the following error: /bin/sh: git:  not found

@ymanton
Copy link
Member

ymanton commented Mar 1, 2023

Jenkins test sanity.functional aix jdk11

@mpirvu
Copy link
Contributor Author

mpirvu commented Mar 2, 2023

All tests have 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.

None yet

2 participants