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

-W max-table-elements has no effect for wasmtime serve #8504

Closed
dicej opened this issue Apr 29, 2024 · 2 comments · Fixed by #8525
Closed

-W max-table-elements has no effect for wasmtime serve #8504

dicej opened this issue Apr 29, 2024 · 2 comments · Fixed by #8525

Comments

@dicej
Copy link
Contributor

dicej commented Apr 29, 2024

While working on resource support for the C# wit-bindgen generator, I created an example app targeting wasi:http/incoming-handler@0.2.0. However, wasmtime serve couldn't run it due to one of the tables containing more than 10000 entries:

 $ wasmtime serve target/stripped.wasm 
Error: table index 0 has a minimum element size of 10511 which exceeds the limit of 10000

Next, I tried wasmtime serve -W max-table-elements=20000 target/stripped.wasm instead, but got the same error. Looking at the code, it seems that the pooling allocator is enabled by default for wasmtime serve, and since it has its own configuration knobs which are not controlled by e.g. -W max-table-elements, there's no way to change the limit from its default. There's also no way I can see to disable the pooling allocator, given that the only reference to the --pooling-allocator option is in old_cli.rs, which is disabled in the default build.

I'd suggest that -W options such as max-table-elements should be applied to the pooling allocator when it is enabled. We might also consider raising the default considering that even simple C# apps tend to exceed it (although we could instead consider that a bug in the C# Native AOT compiler, e.g. insufficient dead code elimination?). EDIT: I was building with -c Debug; the release build results in a table about half the size, which is well under the current default limit.

@dicej
Copy link
Contributor Author

dicej commented Apr 29, 2024

As I noted in my edit above, building the C# app using -c Release ensured that the max table size in the component was well under the default limit, but then I ran into what appears to be an allocation failure during .NET runtime initialization, possibly due to the default memory pages limit being lower than what the .NET runtime needs. I'm going to test that theory using a custom build and report back here.

@dicej
Copy link
Contributor Author

dicej commented Apr 30, 2024

Indeed, setting the memory pages limit to 1600 (i.e. 100MiB) allows the app the run without trapping.

alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue May 2, 2024
This commit updates the processing of pooling allocator options in
`wasmtime serve`. Previously the pooling allocator was enabled by
default but the options to configure it weren't processed due to how
this default-enable was implemented. The option to enable it by default
for `wasmtime serve`, but only `wasmtime serve`, is now processed
differently in a way that handles various other
pooling-allocator-related options.

Closes bytecodealliance#8504
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue May 2, 2024
This commit doubles the default allowed table elements per table in the
pooling allocator from 10k to 20k. This helps to, by default, run the
module produced in bytecodealliance#8504.
github-merge-queue bot pushed a commit that referenced this issue May 2, 2024
* Double the default allowed table elements

This commit doubles the default allowed table elements per table in the
pooling allocator from 10k to 20k. This helps to, by default, run the
module produced in #8504.

* Update docs on deafults
github-merge-queue bot pushed a commit that referenced this issue May 3, 2024
* Respect pooling allocation options in `wasmtime serve`

This commit updates the processing of pooling allocator options in
`wasmtime serve`. Previously the pooling allocator was enabled by
default but the options to configure it weren't processed due to how
this default-enable was implemented. The option to enable it by default
for `wasmtime serve`, but only `wasmtime serve`, is now processed
differently in a way that handles various other
pooling-allocator-related options.

Closes #8504

* Fix compile of bench api

* Fix test build

* Ignore newly added test as it's flaky
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant