Uh oh!
There was an error while loading. Please reload this page.
gh-117645: Increase WASI stack size from 512 KiB to 8 MiB - #117674
Conversation
Increase also the initial memory from 10 MiB to 20 MiB. Reenable test_dynamic on WASI release build.
vstinner
commented
Apr 9, 2024
!buildbot wasi |
bedevere-bot
commented
Apr 9, 2024
vstinner
commented
Apr 9, 2024
The 8 MiB limit (8 388 608 bytes) is already used by the test.pythoninfo: The value is set at: I'm not sure of the difference between the linker With this change, test_dynamic works as expected (on a release build) with 8 MiB stack. |
vstinner
commented
Apr 9, 2024
@brettcannon: Would you mind to review this change? Are you ok to increase the initial stack from 512 KiB to 8 MiB? |
vstinner
commented
Apr 9, 2024
test_dynamic pass again on "buildbot/wasm32 WASI 8Core PR" (built in release mode). Previously, test_dynamic was failing on this buildbot. |
vstinner
commented
Apr 9, 2024
Note: I also tried to leave the initial stack unchanged (512 KiB), and only increase the memory size (to 20 MiB): |
vstinner
commented
Apr 10, 2024
I'm not convinced that wasmtime can grow the stack and/or the total memory size. So I changed the configure.ac comment. |
encukou
commented
Apr 10, 2024
I think this definitely needs a WASI expert. It looks a bit like tuning generic build settings for the specific buildbot. |
vstinner
commented
Apr 15, 2024
I did another test:
test_dynamic fails with: It confirms that the But it can grow the "total" memory (address space). Example on a build with 40 MiB initial memory: I can allocate 60 MiB without any issue. |
vstinner
commented
Apr 15, 2024
It seems like nobody is available to review this change. I plan to merge my PR at the end of the week, unless someone comes and wants to review it :-) The change reenables a test_dynamic test which is currently always skipped on WASI. IMO it will unlock other use cases which are currently limited by the maximum stack memory. IMO a stack of 512 KiB is too small for Python, and sadly |
ericsnowcurrently
left a comment
There was a problem hiding this comment.
LGTM
Thanks for doing this! I've left one comment, but I'll leave it to you discretion.
Uh oh!
There was an error while loading. Please reload this page.
vstinner
commented
Apr 16, 2024
Merged, thanks for the review @ericsnowcurrently. |
…on#117674) Increase also the initial memory from 10 MiB to 20 MiB. Reenable test_dynamic on WASI build.
Increase also the initial memory from 10 MiB to 20 MiB.
Reenable test_dynamic on WASI release build.