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

Optimize compilespeed #1032

Closed
wants to merge 9 commits into from
Closed

Conversation

rPraml
Copy link
Contributor

@rPraml rPraml commented Sep 20, 2021

Hello, I've tried to improve the compile speed, especially, if many exception handlers are involved (see CodegenTest.testManyExceptionHandlers)

This test takes in original 33s on my machine.

This brings a performance boost down to 3.5s

The "all test time" goes down from about 11:50 to 11:20

@gbrail
Copy link
Collaborator

gbrail commented Sep 23, 2021

This all looks very good to me. I was hoping it'd speed up test execution for Rhino itself, but sadly it didn't really make a difference. Do you or does anyone else have an example of some complex JavaScript that now compiles a bunch faster so that we can document that for the record?

@rPraml
Copy link
Contributor Author

rPraml commented Sep 24, 2021

@gbrail I've investigated much time in optimizing test execution (compile and run speed) and did only find a few places where optimization is done. (see other PRs) Unfortunately, most of them is not really measurable.
I've only found the CodegenTest.testManyExceptionHandlers test, where this change makes a huge difference:

As far as I see, the amount of exception handlers goes into compile speed with a complexity of ~ O(n^3) and the CodegenTest creates 1000 of them. If you double the number, the test takes ~8 times longer.

There were 3 nested for - loops, one of them could be replaced with the binary search, so that you'll get O(n^2 log(n))

All in all, compile speed will only increase if you have many of exception blocks (which may be an edge case)

BTW: You can increase the testexecution speed by factor ~2 if jacoco is disabled.

@tuchida
Copy link
Contributor

tuchida commented Sep 24, 2021

Rhino is not currently testing the try statement in test262.
I added language/statements/try to test262.properties, and found that the number of errors was the same before and after the modification.

language/statements/try 117/195 (60.0%)

@gbrail
Copy link
Collaborator

gbrail commented Oct 8, 2021

This seems safe enough to merge as part of 1.7.14. I'll test it one more time and then do it.

@gbrail
Copy link
Collaborator

gbrail commented Oct 8, 2021

Thank you! I merged this manually because I wanted fewer commits but I wanted to also separate the spotless commits from the real work.

@gbrail gbrail closed this Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants