-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Pathological time spent in register allocation for large function #3523
Comments
Running the script from your repository this file doesn't exist, but when corrected to Can you detail a bit more what you're doing, what's fast, and what's slow? |
The repro has a bug where it isn't loading the "fast" wasm file (named With that fixed, I see:
|
Sorry it was my bad, updated fast. Now you should be able to see the difference, I updated the fast one. The context is: We have the moonbase runtime https://github.com/PureStake/moonbeam/blob/master/runtime/moonbase/src/lib.rs that adds https://github.com/PureStake/moonbeam/blob/57fdb3f18fe0a48471bfcad2c37d89708a3a274f/runtime/moonbase/src/lib.rs#L1023. This is moonbase_slow When we inserted this change, we realized an increase on the wasm compilation time. So we are trying to monitor the reason why this happens. For that reason why created this PR moonbeam-foundation/moonbeam#989, with the code commented, compiled it and measured its wasm compilation time. This is moonbase_fast For me the first takes 16 secs, while the latter takes around 1. And I am not able to picture the reason of the difference |
FYI, some debugging I've done indicates that we get into one invocation of |
Thanks for the update, I can indeed reproduce the large discrepancy in compile times. What's happening here is that something pathological is being hit in Cranelift, and it's definitely something that we should fix! Specifically in this wasm file the Extracting just that one function to a wasm module yields extract.wasm.gz and this can be reproduced locally with
There are known pitfalls with our register allocator at this time and plans to move to a better register allocator. My guess is that this is likely to fall into that category of "probably will get fixed", but @cfallin do you think it's worthwhile digging in more here to figure out why something is triggering the slowdown here? @girazoki and @notlesh in the meantime for your specific use case if you'd like to avoid this slowdown I'd recommend taking a look at |
This is a slightly smaller minimization with data strings and import strings all removed as well which makes navigating the |
@alexcrichton That's a good question; ordinarily we'd definitely want to chase something like this, but if the RA2 transition happens soon-ish then the payoff of the time it would take to understand the issue and update the old regalloc, and be confident in the fix, doesn't seem worth it. Especially as many of the details of regalloc.rs have fallen out of my mental L2 cache by now... These sorts of things do put pressure on the transition, though, so hopefully the moving pieces needed for that will come together soon! |
@adamrk given your interest on this issue you might be interested in this one as well (detailed here) but if not no worries! |
Not sure if that would help, but here are the perf (you can open in https://www.speedscope.app/ ) for when it is loading the wasm: https://drive.google.com/file/d/1LSid_h9vs8LGtbaQ8z41CD3vn4-y4g2U/view?usp=sharing |
Sorry, just noticed this. Yeah, I'll take a look at it. |
@alexcrichton We have hit what appears to be the same issue in the Moonbeam project again. This time it is in this wasm file. Following your example, @girazoki, explored this:
How did you figure out the human-readale or even the mangled name of the function to know where to put the |
Oh nice, thanks for that! I was able to find the function-in-question with:
which prints out:
which points to function 1276 as the offender here. Using
which I can confirm the extracted wasm file takes ~20s to compile, quite too long! |
I'm going to close this given the discussion on #4060. The new regalloc probably improves things here but these sorts of outliers are inevitable with Cranelift's design goals. |
Original Issue
We have implemented this repo https://github.com/girazoki/wasmtime-try to try something that has been happening us with some latest code change we have done at moonbeam. The code change is moonbeam-foundation/moonbeam#989.
Without the PR, wasmtime takes around 15 seconds to compile the wasm. However, if we include the change (removal of the trait) wasmtime takes less than a second to compile. We wonder how these differences can be so big with such a small code change.
Both runtimes are in https://github.com/girazoki/wasmtime-try to try them out
Any help would be appreciated, thanks!
Current status
Modules with way-too-long-compile-times:
The text was updated successfully, but these errors were encountered: