-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[wasm] Implement MINT_SWITCH opcode in jiterpreter #107423
Conversation
Introduce runtime option for max switch size Disable trace generation once the trace table fills up, since there's no point to it
Tagging subscribers to this area: @BrzVlad, @kotlarmilos |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BrzVlad could you please review the logic please ?
mono_assert(getU16(ip) === MintOpcode.MINT_SWITCH, "decodeSwitch called on a non-switch"); | ||
const n = getArgU32(ip, 2); | ||
const result = []; | ||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove dead code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not dead code
* Implement MINT_SWITCH opcode (without support for backward jumps) * Introduce runtime option for max switch size (set to 0 to disable switches) * Disable trace generation once the trace table fills up, since there's no point to it
* Implement MINT_SWITCH opcode (without support for backward jumps) * Introduce runtime option for max switch size (set to 0 to disable switches) * Disable trace generation once the trace table fills up, since there's no point to it
This PR implements basic support for switches in the jiterpreter, with a configurable size limit (big ones would eat up all the available trace space) that can be set to 0 in order to disable them.
Switch targets that go backwards (Vlad says these should be exceedingly rare) will cause a runtime bailout, as will switches that are too big. In local testing these bailouts are very rare, even with the current low threshold.
If a switch targets places outside of the trace, only those switch targets that are outside will cause bailouts - the rest of them will still work. So even for large traces where half the switch arms aren't reachable, this is still an improvement over how it was before, I think.
In the future if we raise the max trace size we can also raise the size limit on switches - from looking at instrumentation, a limit of 64 or so is probably ideal.
This PR also fixes a bug I noticed - if the jiterpreter trace table filled up we were still generating+compiling a trace and then failing to put it into the table, which wasted CPU time.