omarjatoi requested wasmtime-core-reviewers for a review on PR #9447.
omarjatoi requested fitzgen for a review on PR #9447.
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
PoolingAllocatorConfiguration
to theOptimizeOptions
struct- Updates setting when
pooling-allocator
is enabled- Fixes
pooling_decommit_batch_size
type tousize
- Rename
memory_protection_keys
topooling_memory_protection_keys
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
PoolingAllocatorConfiguration
to theOptimizeOptions
struct- Set configuration when
pooling-allocator
is enabled and flags are passed- Fixes
pooling_decommit_batch_size
type tousize
- Rename
memory_protection_keys
topooling_memory_protection_keys
omarjatoi submitted PR review.
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
omarjatoi edited PR review comment.
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?
fitzgen created PR review comment:
This one doesn't mention the default value, but it should to be consistent with the other config knobs.
fitzgen submitted PR review:
Thanks! Just a couple comments below
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!
omarjatoi submitted PR review.
omarjatoi created PR review comment:
Yeah, I messed up my commits :sweat_smile: I'll fix them in the PR shortly
fitzgen submitted PR review.
fitzgen created PR review comment:
At this point its fine, no worries, just something to think about for the future :)
omarjatoi updated PR #9447.
omarjatoi updated PR #9447.
omarjatoi commented on PR #9447:
Thanks for the comments! I've added defaults in comments and cleaned up the commits.
fitzgen submitted PR review:
Thanks!
fitzgen has enabled auto merge for PR #9447.
fitzgen merged PR #9447.
Last updated: Dec 23 2024 at 12:05 UTC