Fix/speed regression #705
Merged
Fix/speed regression #705
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Prepare release 10.0.7
guillep
approved these changes
Oct 20, 2023
| cogit cog: newMethod selector: objectMemory nilObject. | ||
| methodHeader := self rawHeaderOf: newMethod ] | ||
| ifFalse: [ self maybeFlagMethodAsInterpreted: newMethod ] ]. | ||
|
|
There was a problem hiding this comment.
Good, do not compile on method activation
| instructionPointer := cogit ceReturnToInterpreterPC]. | ||
| ^ self activateCoggedNewMethod: inInterpreter]. | ||
|
|
||
| "We are in the interpreter" |
There was a problem hiding this comment.
yes, activate compiled method if already compiled
| "if not primitive, or primitive failed, activate the method" | ||
| self activateNewMethod ] | ||
| ] | ||
|
|
There was a problem hiding this comment.
This is the major change in here, now this method can be parametrised to eagerly compile methods (or not). By default the interpreter will not do it.
Merged
|
Thanks for the explanation @guillep |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Just to give some more details (@guillep editing here):
This release fixes a regression introduced during a refactoring.
The refactoring introduced accidentally an eager compilation of methods to machine code and reduced the number of JIT'ted methods activated.
This provoked, on the one hand, spending extra time on compiling useless methods and trashing the machine code cache, and on the other hand, an over-reliance on the interpreter interpreter.
This PR re-introduces those heuristics while keeping the refactoring (that removed lots of duplicated code).
This week we worked also on moving forward the benchmarking infrastructure to be able to eagerly detect these kind of issues in the future. See the list of benchmarks below showing the performance of a couple of benchmarks for VMs version 10.0.4 (default in Pharo 10), 10.0.7 (default in Pharo 11) and 10.0.8 (10.0.7 + this fix).
Interestingly, the regression only affects two benchmarks but very markingly: see column 10.0.7.
Also, since 10.0.7 we have a slight improvement in reverse complement and file benchmarks, probably due to ephemerons and GC improvements.