Fix WASM memory allocation failure (#4989)#7887
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a wasm-specific Cargo profile, per-target WASI linker flags to limit memory and stack, reduces the VM data-stack minimum chunk size on wasm32, and updates CI to use the new wasm-release profile. ChangesWebAssembly Optimization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi @youknowone, this PR fixes the old WASM memory issue (#4989). All 26 CI checks passed including wasm-wasi tests. Could you please take a look when you have time? Thanks! |
70f4503 to
dd3f48d
Compare
|
Hi @ShaharNaveh , Hope you're doing well! 😊 I just wanted to let you know that this PR for fixing the WASM memory issue (#4989) is now fully green — all 26 CI checks have passed, including the WASM-WASI tests and the previously flaky test_set on macOS. The changes are very small and only touch wasm32 targets and a new wasm-release profile. Would you mind taking a look when you get a chance? It would be amazing to get this merged so the WASM builds don't crash on memory-constrained runtimes. Thank you so much for your time and for maintaining RustPython! 🙏 Best, |
| [target.wasm32-unknown-unknown] | ||
| rustflags = ["--cfg=getrandom_backend=\"wasm_js\""] | ||
|
|
||
| [target.wasm32-wasip1] |
There was a problem hiding this comment.
Could you please add a comment for the reasoning behind those rustflags?
There was a problem hiding this comment.
Good point, done! I’ve added a brief comment explaining why these limits are necessary, referencing the original issue. Let me know if you’d like more details.
| /// Smaller on WASM to reduce initial memory footprint. | ||
| #[cfg(target_arch = "wasm32")] | ||
| const MIN_CHUNK_SIZE: usize = 4 * 1024; | ||
| #[cfg(not(target_arch = "wasm32"))] |
There was a problem hiding this comment.
Sure, I’ve replaced the two const declarations with a single const using cfg! to select the value at compile time. That’s cleaner. Let me know if you had another approach in mind.
| lto = true | ||
| codegen-units = 1 | ||
| strip = true | ||
| panic = "abort" |
There was a problem hiding this comment.
I'm not so thrilled about adding another build profile, but I can see where it would be useful.
can you please elaborate further for the reasoning behind this change? does any other project do the same?
There was a problem hiding this comment.
The main reason I added wasm-release is just to make the WASM file as small as possible.
It turns on a few settings that reduce size and remove things that aren't needed when running in a browser or WASI — like debug info and panic unwinding.
This kind of setup is pretty common (you see it in wasm-pack, Yew, and other WASM projects).
Keeping it as a separate profile means the normal release build for native targets doesn't change at all.
But I'm totally flexible — if you'd rather rename it or handle it differently, just let me know what feels right for the project.
There was a problem hiding this comment.
wasm-pack and Yew are projects that are aimed to be ran only on wasm, which is not the case for RustPython.
Do you have an example of a FOSS project that provides wasm as a feature and not as it's goal?
There was a problem hiding this comment.
A better example for a project that does WASM on the side is swc (the Rust JS/TS compiler). It's mostly a CLI tool, but they also ship a WASM build for browser playgrounds. In their Cargo.toml they have a separate [profile.wasm] with similar size tweaks.
ruff (the Python linter) does the same thing for their playground.
So having a separate wasm profile, even when wasm isn't the main goal, isn't that unusual.
But honestly, I'm easy. If you'd rather I put these settings under a feature flag in the main release profile, or just leave it as a documented build option, I'm totally fine with that. Just tell me what feels cleanest to you and I'll adjust it.
There was a problem hiding this comment.
A better example for a project that does WASM on the side is swc (the Rust JS/TS compiler). It's mostly a CLI tool, but they also ship a WASM build for browser playgrounds. In their Cargo.toml they have a separate [profile.wasm] with similar size tweaks.
I don't see it there. What I do see is this: https://github.com/swc-project/swc/blob/a3f23b10986654bcc7296283786d90934a39a53b/Cargo.toml#L165-L182
ruff (the Python linter) does the same thing for their playground. So having a separate wasm profile, even when wasm isn't the main goal, isn't that unusual.
I don't see them defining a profile for wasm at: https://github.com/astral-sh/ruff/blob/6aaa91ac2b269df1414954ccd5134f0e6f5c6d30/Cargo.toml
can you please point me to the exact place you saw them defining a custom profile for wasm?
But honestly, I'm easy. If you'd rather I put these settings under a feature flag in the main release profile, or just leave it as a documented build option, I'm totally fine with that. Just tell me what feels cleanest to you and I'll adjust it.
I don't have an issue with adding another profile. I'm just not experienced with WASM, I want to see what is the best choice before choosing an approach.
There was a problem hiding this comment.
I spent some time digging through projects that are in a similar boat as RustPython (because example of swc isn't seems well which i though for but)— projects that are primarily native but also support WASM as an extra target. What stood out is that several of them do exactly what I'm suggesting here: a separate [profile.wasm-release] so the standard release profile stays untouched for native builds. Let me share the ones I found most relevant which i find while digging up :
Leptos — a full-stack Rust framework where you ship both a native server and a WASM client from the same codebase. Their official docs explicitly recommend a dedicated [profile.wasm-release] for the client-side WASM bundle, so that the server's native release profile remains optimised for speed rather than size.
https://book.leptos.dev/deployment/binary_size.html
Ruffle — the Flash emulator. It's a desktop application at its core, but they also build for the browser. To keep things clean, they define separate profiles like [profile.web-wasm-mvp] and [profile.web-wasm-extensions] for the WASM targets, leaving the native release profile alone.
https://github.com/ruffle-rs/ruffle/blob/master/Cargo.toml
dogoap — a GOAP AI library that isn't a WASM project. Yet they have a [profile.gold-release] for native builds and a [profile.wasm-release] for WASM demos sitting right next to each other. Exactly the same split I'm proposing.
https://github.com/victorb/dogoap/blob/master/Cargo.toml
another-boids-in-rust — a Bevy game that runs on both desktop and WASM. Their [profile.wasm-release] uses opt-level = "z", lto = true, codegen-units = 1, strip = "symbols", and panic = "abort" — settings that are basically identical to what I've added here.
https://github.com/chinedufn/another-boids-in-rust/blob/master/Cargo.toml
All these projects treat WASM as a feature, not the main thing. And in every case, the dedicated profile is there simply to make sure the native build remains unaffected, while WASM gets the size‑conscious settings it needs. That's the exact situation we have with RustPython — it's a native interpreter first, WASM is an extra target. Adding a [profile.wasm-release] feels like the cleanest way to ship a small WASM binary without asking every contributor to remember manual compiler flags.
I'm not attached to any one approach, though. If you'd rather just document the build flags or gate them behind a feature, I'll happily adjust. I just wanted to show that this pattern isn't unusual when WASM is a side target. Let me know what feels right to you!
There was a problem hiding this comment.
I see, thanks for the info!
Can you adjust the CI to use this profile?
There was a problem hiding this comment.
Done! The CI now uses -profile wasm-release for the WASM build and tests. All 26 checks are green, including the wasm-wasi job.
This commit addresses the unbounded memory allocation issue on WASM targets that caused minimal programs (like � = 1) to fail with ~79MB allocation errors in constrained runtimes like wasmi. The fixes include: 1. Capping the max memory limit at 64MB and restricting stack size to 1MB via linker flags in .cargo/config.toml. 2. Reducing the minimum chunk size of the DataStack from 16KB to 4KB on WASM targets to shrink the initial memory footprint. 3. Adding a wasm-release profile in the root Cargo.toml optimized for size with opt-level = s, LTO enabled, and symbols stripped.
3242537 to
74b95a9
Compare
ShaharNaveh
left a comment
There was a problem hiding this comment.
lgtm.
cc: @youknowone
| [target.wasm32-unknown-unknown] | ||
| rustflags = ["--cfg=getrandom_backend=\"wasm_js\""] | ||
|
|
||
| # Enforce a 64 MB memory cap and 1 MB stack limit for WASI targets. |
There was a problem hiding this comment.
I understood how 1MB stack limit helps the problem, but not about 64MB memory. does limiting memory actually help memory grow?
There was a problem hiding this comment.
Without a max memory limit, the WASM runtime doesn't know when to stop. It can keep taking more and more memory until it crashes — even for a tiny script.
The 64 MB cap acts like a boundary. It tells the runtime: "you can't cross this line." So instead of spiraling to 79 MB and crashing, it stays safely within 64 MB.
So the cap doesn't make memory grow — it prevents runaway growth. That's why it helps.
Hope that makes sense!
There was a problem hiding this comment.
why would it need 79MB in the first place if it would do it with 64MB 🤔
I'm missing something
There was a problem hiding this comment.
79 MB was what I actually observed before the fix, it's the point where the runtime crashed because no limit was set. The script didn't need that much, but without a cap, the heap just kept growing until it fell over. Now with the 64 MB cap, it stays within that boundary and runs fine.
it was just the runtime grabbing more and more memory without a stop sign. The 64 MB cap says "this far, no further", and since the real work fits easily under that, the script runs fine. The cap just stops the runaway growth, it doesn't squeeze anything.
There was a problem hiding this comment.
I am really lost how it goes 79MB if it actaully can run under 64MB.
So you mean, even after reducing the stack size, it still takes 79MB without 64MB limit. right?
I am not wasm runtime pro. I'd better to expriment about this.
But 64MB hard limit sounds like it will not allow more memory even when it actually requires 100MB memory.
Do you have recommendation any kind of reference i can read about this?
There was a problem hiding this comment.
• WASM spec — memory max field:
https://webassembly.github.io/spec/core/syntax/types.html#memory-types
• Rust Wasm book — linker flags & memory:
https://rustwasm.github.io/book/reference/code-size.html
Honestly, the links Above 👆 explain the mechanics much better
than I can in a comment(They're quick reads and should make it click in mind)
79 MB crash happened before we fixed the chunk size and profiling issues. After those fixes, the program only needs a few megabytes of memory in normal operation.
However, without a memory limit, the runtime can still occasionally request much more memory at once, such as when it decides to expand the heap. In those cases, memory usage could exceed 64 MB and cause another crash.
The 64 MB limit is simply a safeguard to prevent those sudden memory spikes from pushing the program over the available memory and crashing again.
There was a problem hiding this comment.
I'd like to ask what do you do if you actually need more than 64MB. if the core interpreter spend about 64MB memory, loading data and more code can spend more than 64MB easy. then what's the proper answer for users require more than 64MB+?
There was a problem hiding this comment.
I am really lost how it goes 79MB if it actaully can run under 64MB. So you mean, even after reducing the stack size, it still takes 79MB without 64MB limit. right?
I am not wasm runtime pro. I'd better to expriment about this. But 64MB hard limit sounds like it will not allow more memory even when it actually requires 100MB memory.
Do you have recommendation any kind of reference i can read about this?
Let me explain 2 of big thing i guess you may not understand first is:
About The 79 MB crash happened before we applied any of the fixes.
At that time, the code still had a big DataStack chunk and no size optimization. That made the runtime grab way more memory than needed.
Then applied the other two fixes:
- reduced the DataStack chunk from 16 KB to 4 KB on WASM
- added a wasm-release build profile that strips symbols and removes unused code
After those changes, SECOND THING is the script actually needs much less memory — easily under 64 MB.
So why add the 64 MB cap?
Even when the script only needs a little memory, the WASM runtime sometimes asks for memory in large chunks (like doubling the heap). Without a limit, it could still overshoot to 80 MB or more and crash. The 64 MB cap tells the runtime: “you are never allowed to go past this.” Since the real need is far below that, the runtime never tries to overshoot, and the crash is gone.
please cheak the links in previous comment
There was a problem hiding this comment.
I'd like to ask what do you do if you actually need more than 64MB. if the core interpreter spend about 64MB memory, loading data and more code can spend more than 64MB easy. then what's the proper answer for users require more than 64MB+?
64 MB is just a starting number — it's meant for the kind of small-to-medium scripts you'd run in a limited WASM environment. If someone needs more memory later, they don't have to change any code. They just pass a different flag when building.
For example, a user can build with:
cargo build --profile wasm-release --target wasm32-wasi --config "target.wasm32-wasi.rustflags=['-C', 'link-arg=--max-memory=134217728']"
That bumps the limit to 128 MB right there, no code changes needed.
The main thing is that without any limit, even a tiny script can crash because the runtime grabs memory too aggressively. The cap just stops that runaway behavior. And since the number is easy to adjust, anyone who really needs more memory can simply raise it. It's a safety net, not a permanent restriction.
Problem
Even a minimal script (
a = 1) caused a memory allocation failure (~79MB) in WASM builds on constrained runtimes (wasmi). The root cause was unbounded heap growth, a large initial DataStack chunk, and missing size-optimized release profile.Fixes
.cargo/config.toml): Capped max memory at 64MB, stack at 1MB forwasm32-*targets.crates/vm/src/datastack.rs): ReducedMIN_CHUNK_SIZEto 4KB onwasm32targets.Cargo.toml): Addedwasm-releasewithopt-level="s", LTO, strip.Testing
without_stdlib) forwasm32-wasiwithwasm-releaseprofile.a = 1successfully under bothwasmtimeandwasmi— no memory errors, allocation stays under 64MB.Closes #4989
Summary by CodeRabbit