Skip to content

Fix WASM memory allocation failure (#4989) #7887

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.

Sign up for GitHub

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

Merged
youknowone merged 11 commits into from
Jun 3, 2026
+31 −4
Merged

Fix WASM memory allocation failure (#4989) #7887

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
3ae73f0
Fix WASM memory allocation failure (#4989)
alok-108 May 16, 2026
09aba03
Trigger CI rerun
alok-108 May 23, 2026
2808d1f
Address review feedback: add comments, use cfg! for MIN_CHUNK_SIZE
alok-108 May 24, 2026
f084aba
Remove duplicate MIN_CHUNK_SIZE definition
alok-108 May 24, 2026
2be3382
Fix duplicate MIN_CHUNK_SIZE and format for prek lint
alok-108 May 25, 2026
4c4f4ee
Update CI to use wasm-release profile for WASM tests
alok-108 May 27, 2026
620f31d
Remove leftover conflict markers
alok-108 May 28, 2026
eeb0dad
Clean remaining conflict markers in CI workflow
alok-108 May 28, 2026
74b95a9
Clean remaining conflict markers in CI workflow
alok-108 May 28, 2026
407890c
Trigger CI to pick up new required check
alok-108 Jun 2, 2026
d9245f7
Remove --max-memory flag, keep stack limit only
alok-108 Jun 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .cargo/config.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,16 @@ rustflags = "-C link-args=-Wl,--stack,8000000"

[target.wasm32-unknown-unknown]
rustflags = ["--cfg=getrandom_backend=\"wasm_js\""]

# Enforce a 1 MB stack limit for WASI targets.
# (Runaway heap growth is fixed by the smaller DataStack chunk
# and the size‑optimized wasm‑release profile.)
[target.wasm32-wasip1]

@ShaharNaveh ShaharNaveh May 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a comment for the reasoning behind those rustflags?

@alok-108 alok-108 May 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

rustflags = [
"-C", "link-arg=-zstack-size=1048576",
]

[target.wasm32-wasip2]
rustflags = [
"-C", "link-arg=-zstack-size=1048576",
]
7 changes: 4 additions & 3 deletions .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -790,11 +790,11 @@ jobs:
clang: true

- name: build rustpython
run: cargo build --release --target wasm32-wasip1 --no-default-features --features freeze-stdlib,stdlib,stdio,importlib,host_env --verbose
run: cargo build --profile wasm-release --target wasm32-wasip1 --no-default-features --features freeze-stdlib,stdlib,stdio,importlib,host_env --verbose
- name: run snippets
run: wasmer run --dir "$(pwd)" target/wasm32-wasip1/release/rustpython.wasm -- "$(pwd)/extra_tests/snippets/stdlib_random.py"
run: wasmer run --dir "$(pwd)" target/wasm32-wasip1/wasm-release/rustpython.wasm -- "$(pwd)/extra_tests/snippets/stdlib_random.py"
- name: run cpython unittest
run: wasmer run --dir "$(pwd)" target/wasm32-wasip1/release/rustpython.wasm -- "$(pwd)/Lib/test/test_int.py"
run: wasmer run --dir "$(pwd)" target/wasm32-wasip1/wasm-release/rustpython.wasm -- "$(pwd)/Lib/test/test_int.py"

cargo_doc:
needs:
Expand Down Expand Up @@ -831,3 +831,4 @@ jobs:

- name: cargo doc
run: cargo doc --locked

8 changes: 8 additions & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
flamescope = { version = "0.1.2", optional = true }

rustls = { workspace = true, optional = true }
rustls-graviola = { workspace = true, optional = true }

Check warning on line 52 in Cargo.toml

View workflow job for this annotation

GitHub Actions / cargo shear

shear/misplaced_optional_dependency 

misplaced optional dependency `rustls-graviola` (remove the `optional` flag and move to `[dev-dependencies]`)

[target.'cfg(windows)'.dependencies]
libc = { workspace = true }
Expand Down Expand Up @@ -117,6 +117,14 @@
[profile.release]
lto = "thin"

[profile.wasm-release]
inherits = "release"
opt-level = "s"
lto = true
codegen-units = 1
strip = true
panic = "abort"

@ShaharNaveh ShaharNaveh May 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@alok-108 alok-108 May 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@ShaharNaveh ShaharNaveh May 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@alok-108 alok-108 May 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@ShaharNaveh ShaharNaveh May 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@alok-108 alok-108 May 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

@ShaharNaveh ShaharNaveh May 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks for the info!

Can you adjust the CI to use this profile?

@alok-108 alok-108 May 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! The CI now uses -profile wasm-release for the WASM build and tests. All 26 checks are green, including the wasm-wasi job.


[patch.crates-io]
parking_lot_core = { git = "https://github.com/youknowone/parking_lot", branch = "rustpython" }
# REDOX START, Uncomment when you want to compile/check with redoxer
Expand Down
7 changes: 6 additions & 1 deletion crates/vm/src/datastack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ use core::alloc::Layout;
use core::ptr;

/// Minimum chunk size in bytes (`_PY_DATA_STACK_CHUNK_SIZE`).
const MIN_CHUNK_SIZE: usize = 16 * 1024;
/// Smaller on WASM (4 KB) to reduce initial memory footprint; 16 KB otherwise.
const MIN_CHUNK_SIZE: usize = if cfg!(target_arch = "wasm32") {
4 * 1024
} else {
16 * 1024
};

/// Extra headroom (in bytes) to avoid allocating a new chunk for the next
/// frame right after growing.
Expand Down
Loading