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

[interp] Split interpreter into recursive and non-recursive parts, again. #33336

Closed

Conversation

monojenkins
Copy link
Contributor

!! This PR is a copy of mono/mono#19148, please do not edit or review it in this repo !!
Do not automatically approve this PR:

* Consider how the changes affect configurations in this repo,
* Check effects on files that are not mirrored,
* Identify test cases that may be needed in this repo.

!! Merge the PR only after the original PR is merged !!



The latest reason for this, is that the non-recursive interpreter
stresses JITs, in that it requires either:

  • a first tier optimizing JIT -- which is expected on iOS soon (now?)
  • or on-stack-replacement -- which is expected to exists, but maybe not, or benchmarks miss it?
  • or maybe a warm-up phase -- seems bad

This requires none of that.

Still to test it and see if it works with iOS.

@danmoseley
Copy link
Member

Good bot labeling mono correctly!

@monojenkins monojenkins force-pushed the sync-pr-19148-from-mono branch 3 times, most recently from 26efb90 to 07da48b Compare March 9, 2020 08:47
@monojenkins monojenkins force-pushed the sync-pr-19148-from-mono branch 2 times, most recently from ef46cbf to d945be9 Compare March 11, 2020 09:18
@monojenkins monojenkins force-pushed the sync-pr-19148-from-mono branch 3 times, most recently from a3293e0 to 90f9144 Compare March 11, 2020 09:40
…small.

This has been brought up and implemented countless times already.

The latest reason for this, is that the non-recursive interpreter
stresses JITs, in that it requires either:
 - a first tier optimizing JIT -- which is expected on iOS soon (now?)
 - or on-stack-replacement -- which is expected to exists, but maybe not, or benchmarks miss it?
    And possibly current-function replacement? Does that occur?
 - or maybe a warm-up phase -- seems bad

This improves WebAssembly performance by 12% as measured with
`run-benchmark-wasm.sh bilinear`.

This has been verified to also address the stack overflow problem
 with current Blazor release from around early January 2020,
 which master's non-recursive interpreter also does.

I will record here some of the benchmarks.
These appear to avoid the tiering issue, but yet show this to be faster.
This data all hand transcribed instead of copy/pasted, sorry.

master (mono/mono@488da04):
 bilinear avg:27097 min:27053 max:27200
 segmentation avg:17668 min:17623 max:17720
 poker avg:6981 min:6953 max:7035
 exceptions avg:0.0514 min:0.0457 max:0.3736

PR (from mono/mono@9ce7798):
 bilinear avg:23568 min:23509 max:23749 (avg 13% faster)
 segmentation avg:14535 min:14406 max:14898 (avg 17% faster)
 poker avg:6079 min:6038 max:6144 (avg 13% faster)
 exceptions avg:0.0356 min:0.0334 max:0.1859 (avg 30% faster)

For non-wasm, non-recursive is faster.

master poker avg:1345 1546 1501 (multiple runs)
PR poker avg:1662 1737 1699 1790 (multiple runs)

master bilinear avg:4317
PR bilinear avg:6178 6104

master segmentation avg:3117 3107 (multiple runs)
PR segmentation avg:3910 3690 (multiple runs)

master exceptions avg:0.00459
PR exceptions avg:0.00446
@monojenkins monojenkins force-pushed the sync-pr-19148-from-mono branch from 90f9144 to ac1c668 Compare March 11, 2020 10:45
@lambdageek lambdageek closed this Mar 20, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants