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

Make SVM generate few enough records to enable it during startup #3752

Closed
28 tasks done
jdmpapin opened this issue Nov 20, 2018 · 6 comments
Closed
28 tasks done

Make SVM generate few enough records to enable it during startup #3752

jdmpapin opened this issue Nov 20, 2018 · 6 comments

Comments

@jdmpapin
Copy link
Contributor

jdmpapin commented Nov 20, 2018

The symbol validation manager (SVM), introduced in #2919 and eclipse-omr/omr#2978, allows ahead of time (AOT) compilations to inspect more run-time state than they were previously allowed to, and more completely captures the conditions (conservatively) required in order to load and run the generated code. As a result, it generates more validation records than the previous approach did—the total number of records generated, including (non-validation) relocations, can be up to 3× the prior count. Because the increase is so large, it is liable to regress startup time by exhausting the available space for AOT-compiled code, significantly reducing the number of methods loaded. As such, the SVM has been disabled during startup (394655f) from the outset.

I have implemented a series of changes to the SVM, consisting first of cleanup (factoring duplicated code, deleting unused code, improving the assertions, etc.), followed by improvements that reduce the number of generated records. All told, these changes bring the number of records down by 33-50% to approximately 1.5× the original amount, which is hopefully low enough to prevent the SVM from causing a startup regression (although that will depend on the specific workload in question). There are quite a few changes, and this issue is to track them. Below I've included a rough dependency graph of my planned pull requests, which I will update as necessary to respond to merges and any requested changes (or just to fix errors, e.g. to add dependencies that I've missed). Many of the dependencies are incidental, due to the fact that the code must be changed in some particular order.

Grey filled nodes have open pull requests. The dashed edges are only an ordering preference, not a real dependency.

@jdmpapin
Copy link
Contributor Author

@andrewcraik @dsouzai @mstoodle FYI

@andrewcraik
Copy link
Contributor

@jdmpapin thank you for taking on this big chunk of work - it certainly looks like it will be a great improvement to the AOT framework. I know that @dsouzai is working with some of the test folks to see if we can get a test bucket for AOT sorted so we can use that to test the pull requests (which is why I'm holding off on merging a just now until that discussion resolves).

@dsouzai
Copy link
Contributor

dsouzai commented Nov 21, 2018

I've opened #3774 to track improving the AOT infrastructure. Adding new modes looks to be an ongoing process. However, I will address the "Update Shared Classes Test to run twice" issue which will, at the very least, improve the AOT testing of the existing Shared Classes test found in the systemTest bucket.

@dsouzai
Copy link
Contributor

dsouzai commented Nov 21, 2018

adoptium/openj9-systemtest#54 is merged, running the SCC tests in the systemTest bucket will result in two runs before destroying the SCC.

@fjeremic
Copy link
Contributor

fjeremic commented Feb 8, 2019

FYI if you use ZenHub you don't need the graph of dependencies since it supports issue dependency chains out of the box.

@jdmpapin
Copy link
Contributor Author

jdmpapin commented Feb 14, 2019

#4723 has been merged 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants