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

Reduce AOT load failures with SVM enabled #10644

Merged
merged 3 commits into from Sep 18, 2020

Conversation

dsouzai
Copy link
Contributor

@dsouzai dsouzai commented Sep 17, 2020

#4897

This PR introduces some improvements to the SVM to reduce AOT load failures caused by validation failures:

  1. Refactor isClassWorthRemembering to facilitate adding more classes in the future
  2. Add java/lang/StringBuffer to the list of classes not worth remembering. The motivation behind this is newer java code is moving towards using StringBuilder. Therefore, ignoring this class reduces load failures, while having minimal impact on steady state throughput.
  3. Check if the class is worth remembering in the addMethodRecord and addClassInfoIsInitialized before creating the validation records. In the case of the former, several validation failures were related to the <init> method of exception classes; exception classes are already part of the classes not worth remembering, so the <init> method follows suit.

With these changes, the number of AOT load failures with the SVM reduces by more than half; however, no SVM AOT still has less load failures (~50). Additionally, the startup regression between JDK11 SVM compared to the default JDK11 NoSVM is reduced to as much as 1.5% (though this seems to vary depending on the day). The throughput hit for these changes is ~1% - this isn't the throughput of a default run, but rather, the code quality of SVM AOT reduces by ~1% (this is mostly due to the StringBuffer change).

Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
Newer java code is moving towards using StringBuilder. Therefore,
ignoring this class reduces load failures, while having minimal
impact on steady state throughput.

Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
addMethodRecord and addClassInfoIsInitialized never checked if the class
(or the class of the method in the case of the former) were worth
remembering. This led to several AOT load failures. Filtering out the
methods not worth remembering in these APIs reduce AOT load failures.

Signed-off-by: Irwin D'Souza <dsouzai.gh@gmail.com>
@dsouzai
Copy link
Contributor Author

dsouzai commented Sep 17, 2020

@mpirvu could you please review? @vijaysun-omr fyi.

@mpirvu
Copy link
Contributor

mpirvu commented Sep 17, 2020

If I understand correctly the proposed changes introduce a 1% throughput regression on runs that use only AOT. If we allow recompilation, that 1% would vanish, correct?

@mpirvu
Copy link
Contributor

mpirvu commented Sep 17, 2020

Another question: while the number of AOT load failures has been reduced, what is the effect on startup time? You mentioned the gap between SVM and non-SVM is 1.5%, but what is the actual start-up time improvement that this change brings to the table?

Copy link
Contributor

@mpirvu mpirvu 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 mpirvu self-assigned this Sep 17, 2020
@mpirvu
Copy link
Contributor

mpirvu commented Sep 18, 2020

jenkins test sanity all jdk11

@dsouzai
Copy link
Contributor Author

dsouzai commented Sep 18, 2020

If I understand correctly the proposed changes introduce a 1% throughput regression on runs that use only AOT. If we allow recompilation, that 1% would vanish, correct?

I believe so yeah; I haven't explicitly done a traditional throughput run, but given that in, say OpenLiberty, the SCC is going to be ~80M, I don't think there's going to be enough AOT code for there to be an impact. However, even if we ran a modified throughput setup, where we specified forceAOT and ran with a bigger SCC but allowed recomp, I believe that 1% should vanish.

Another question: while the number of AOT load failures has been reduced, what is the effect on startup time? You mentioned the gap between SVM and non-SVM is 1.5%, but what is the actual start-up time improvement that this change brings to the table?

Currently during startup we have SVM disabled because in JDK11, SVM enabled regresses startup time by ~3-4%. With these changes, it is still regressed, though now as low as 1.5%.

@mpirvu mpirvu merged commit c363c49 into eclipse-openj9:master Sep 18, 2020
dmitry-ten added a commit to dmitry-ten/openj9 that referenced this pull request Sep 25, 2020
PR eclipse-openj9#10644 added a static array of classes not worth remembering
to Symbol Validation Manager. JITServer, however, requires this array
to be stored on a per client bases, since pointers to J9 classes
are different between clients.

Signed-off-by: Dmitry Ten <Dmitry.Ten@ibm.com>
dmitry-ten added a commit to dmitry-ten/openj9 that referenced this pull request Sep 28, 2020
PR eclipse-openj9#10644 added a static array of classes not worth remembering
to Symbol Validation Manager. JITServer, however, requires this array
to be stored on a per client bases, since pointers to J9 classes
are different between clients.

Signed-off-by: Dmitry Ten <Dmitry.Ten@ibm.com>
@dsouzai dsouzai deleted the reloFailure branch January 4, 2021 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants