Wasmtime GitHub notifications bot (Oct 10 2024 at 17:28): Wasmtime GitHub notifications bot (Oct 10 2024 at 17:28):omarjatoi requested wasmtime-core-reviewers for a review on PR #9447.
Wasmtime GitHub notifications bot (Oct 10 2024 at 17:28):omarjatoi requested fitzgen for a review on PR #9447.
Wasmtime GitHub notifications bot (Oct 10 2024 at 17:28):omarjatoi opened PR #9447 from omarjatoi:9256-pooling-allocator-flags to bytecodealliance:main:
From #9256, this PR adds the missing pooling allocator configuration options as CLI flags. I also renamed some minor things for consistency.
- Adds missing
PoolingAllocatorConfigurationto theOptimizeOptionsstruct- Updates setting when
pooling-allocatoris enabled- Fixes
pooling_decommit_batch_sizetype tousize- Rename
memory_protection_keystopooling_memory_protection_keys
Wasmtime GitHub notifications bot (Oct 10 2024 at 17:31):omarjatoi edited PR #9447:
From #9256, this PR adds the missing pooling allocator configuration options as CLI flags. I also renamed some minor things for consistency.
- Adds missing
PoolingAllocatorConfigurationto theOptimizeOptionsstruct- Set configuration when
pooling-allocatoris enabled and flags are passed- Fixes
pooling_decommit_batch_sizetype tousize- Rename
memory_protection_keystopooling_memory_protection_keys
Wasmtime GitHub notifications bot (Oct 10 2024 at 17:37):omarjatoi submitted PR review.
Wasmtime GitHub notifications bot (Oct 10 2024 at 17:37):omarjatoi created PR review comment:
This option was added to the struct but never used below: https://github.com/bytecodealliance/wasmtime/blob/main/crates/wasmtime/src/config.rs#L2585-L2588
Wasmtime GitHub notifications bot (Oct 10 2024 at 17:41):omarjatoi edited PR review comment.
Wasmtime GitHub notifications bot (Oct 10 2024 at 19:02):fitzgen created PR review comment:
Beyond the weird indentation, I think this is just a duplicate copy of what the existing
if let Some(limit)is doing, but nested inside the existing copy?
Wasmtime GitHub notifications bot (Oct 10 2024 at 19:02):fitzgen created PR review comment:
This one doesn't mention the default value, but it should to be consistent with the other config knobs.
Wasmtime GitHub notifications bot (Oct 10 2024 at 19:02):fitzgen submitted PR review:
Thanks! Just a couple comments below
Wasmtime GitHub notifications bot (Oct 10 2024 at 19:02):fitzgen created PR review comment:
Ah, I see that earlier issue is fixed in this later commit.
In the future, can you either keep separate commits clean and individually reviewable or squash them into a single commit so that we don't have confusing situations like this? Thanks!
Wasmtime GitHub notifications bot (Oct 10 2024 at 19:04):omarjatoi submitted PR review.
Wasmtime GitHub notifications bot (Oct 10 2024 at 19:04):omarjatoi created PR review comment:
Yeah, I messed up my commits :sweat_smile: I'll fix them in the PR shortly
Wasmtime GitHub notifications bot (Oct 10 2024 at 19:08):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Oct 10 2024 at 19:08):fitzgen created PR review comment:
At this point its fine, no worries, just something to think about for the future :)
Wasmtime GitHub notifications bot (Oct 10 2024 at 19:21):omarjatoi updated PR #9447.
Wasmtime GitHub notifications bot (Oct 10 2024 at 19:23):omarjatoi updated PR #9447.
Wasmtime GitHub notifications bot (Oct 10 2024 at 19:26):omarjatoi commented on PR #9447:
Thanks for the comments! I've added defaults in comments and cleaned up the commits.
Wasmtime GitHub notifications bot (Oct 10 2024 at 19:33):fitzgen submitted PR review:
Thanks!
Wasmtime GitHub notifications bot (Oct 10 2024 at 19:33):fitzgen has enabled auto merge for PR #9447.
Wasmtime GitHub notifications bot (Oct 10 2024 at 19:50):fitzgen merged PR #9447.
Last updated: Nov 22 2024 at 16:03 UTC