[Merged by Bors] - Use opcode table rather than match #2501
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
Codecov Report
@@ Coverage Diff @@
## main #2501 +/- ##
==========================================
+ Coverage 51.43% 51.59% +0.16%
==========================================
Files 354 354
Lines 35767 35641 -126
==========================================
- Hits 18395 18390 -5
+ Misses 17372 17251 -121
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
jedel1043
requested review from
HalidOdat,
jasonwilliams,
nekevss,
RageKnify,
raskad and
Razican
jedel1043
added
performance
jedel1043
requested changes
Dec 21, 2022
jedel1043
approved these changes
Dec 21, 2022
|
This is awesome @tunz thanks, will take a look when I have time or in the new year |
HalidOdat
approved these changes
Dec 23, 2022
nekevss
approved these changes
Dec 26, 2022
|
bors r+ |
bors bot
pushed a commit
that referenced
this pull request
Dec 26, 2022
`execute_instruction` is heavily used. After decoding an opcode, `match` is used to find a proper `execute` function for the opcode. But, the `match` may not be able to be optimized into a table jump by rust compiler, so it may use multiple branches to find the function. When I tested with a toy program, only `enum -> &'static str` case was optimized to use a table while `enum -> function call` uses multiple branches. ([gotbolt](https://rust.godbolt.org/z/1rzK5vj6f)) This change makes the opcode to use a table explicitly. It improves the benchmark score of Richards by 1-2% (22.8 -> 23.2).
|
Build failed: |
|
bors r+ |
bors bot
pushed a commit
that referenced
this pull request
Dec 26, 2022
`execute_instruction` is heavily used. After decoding an opcode, `match` is used to find a proper `execute` function for the opcode. But, the `match` may not be able to be optimized into a table jump by rust compiler, so it may use multiple branches to find the function. When I tested with a toy program, only `enum -> &'static str` case was optimized to use a table while `enum -> function call` uses multiple branches. ([gotbolt](https://rust.godbolt.org/z/1rzK5vj6f)) This change makes the opcode to use a table explicitly. It improves the benchmark score of Richards by 1-2% (22.8 -> 23.2).
|
Build failed: |
After decoding an opcode, `match` is used to find a proper `execute` function for the opcode. But, the `match` may not be able to be optimized into a linear table jump by rust compiler, so it may use multiple branches to find the function. This change makes the opcode to use a table explicitly. This change improves the benchmark score of Richards about 1-2% (22.8 -> 23.2).
jedel1043
force-pushed
the
tunz/opt-execute-instruction
branch
from
a6e0f8e to
171edd7
Compare
|
Rebased to see if that's what's causing the fail |
|
bors r+ |
bors bot
pushed a commit
that referenced
this pull request
Dec 26, 2022
`execute_instruction` is heavily used. After decoding an opcode, `match` is used to find a proper `execute` function for the opcode. But, the `match` may not be able to be optimized into a table jump by rust compiler, so it may use multiple branches to find the function. When I tested with a toy program, only `enum -> &'static str` case was optimized to use a table while `enum -> function call` uses multiple branches. ([gotbolt](https://rust.godbolt.org/z/1rzK5vj6f)) This change makes the opcode to use a table explicitly. It improves the benchmark score of Richards by 1-2% (22.8 -> 23.2).
|
Pull request successfully merged into main. Build succeeded: |
bors
bot
changed the title
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.
execute_instructionis heavily used. After decoding an opcode,matchis used to find a properexecutefunction for the opcode. But, thematchmay not be able to be optimized into a table jump by rust compiler, so it may use multiple branches to find the function. When I tested with a toy program, onlyenum -> &'static strcase was optimized to use a table whileenum -> function calluses multiple branches. (gotbolt)This change makes the opcode to use a table explicitly. It improves the benchmark score of Richards by 1-2% (22.8 -> 23.2).