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

Respect pooling allocation options in wasmtime serve #8525

Merged

Conversation

alexcrichton
Copy link
Member

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

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 alexcrichton requested a review from a team as a code owner May 2, 2024 07:06
@alexcrichton alexcrichton requested review from fitzgen and removed request for a team May 2, 2024 07:06
@fitzgen fitzgen added this pull request to the merge queue May 2, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 2, 2024
@alexcrichton
Copy link
Member Author

I've made the decision to ignore the test I've added here with #[ignore]. The CI failure shows a bug where we're racing between two events:

  • The wasmtime serve process is printing out error information
  • The test process is running child.kill() to reap status information as the test is over

There's no way to resolve this race with wasmtime serve right now as there is no graceful shutdown of wasmtime serve. I'd like to add that in the future (e.g. ctrl-c waits for all active connections), but that's a bit out of scope for this. In the meantime it still seems good to get a fix in at least.

@alexcrichton alexcrichton enabled auto-merge May 3, 2024 19:33
@alexcrichton alexcrichton added this pull request to the merge queue May 3, 2024
Merged via the queue into bytecodealliance:main with commit dc1d128 May 3, 2024
21 checks passed
@alexcrichton alexcrichton deleted the fix-pooling-allocator-options branch May 3, 2024 20:10
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 this pull request may close these issues.

-W max-table-elements has no effect for wasmtime serve
2 participants