alexcrichton requested wasmtime-core-reviewers for a review on PR #8616.
alexcrichton requested fitzgen for a review on PR #8616.
alexcrichton opened PR #8616 from alexcrichton:remove-another-page-based-variable
to bytecodealliance:main
:
This commit changes the wasm-page-sized
static_memory_bound
field to instead being a byte-defined unit rather than a page-defined unit. To accomplish this the field is renamed tostatic_memory_reservation
and all references are updated. This builds on the support from #8608 to remove another page-based variable from the internals of Wasmtime.<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
alexcrichton updated PR #8616.
abrown submitted PR review:
Looks good to me! I think consistently using bytes is just better; when I was mucking around in the pooling allocator it was always tricky to understand whether variables were page-based or byte-based. I remember having to handle a lot of overflow conditions which I found by
proptest
-ing... I assume that was handled somewhere else?
alexcrichton commented on PR #8616:
Yeah I think most of those were handled in https://github.com/bytecodealliance/wasmtime/pull/8608 although there's still
memory_pages
in the pooling allocator which I'd like to remove as well and is probably also a large source of overflow checks.
fitzgen commented on PR #8616:
I think consistently using bytes is just better
Agreed, especially since it is effectively necessary to support the custom-page-sizes proposal.
when I was mucking around in the pooling allocator it was always tricky to understand whether variables were page-based or byte-based
I think we should probably start newtyping our units. Way back when I wrote some stuff like this for
wee_alloc
and then someone pulled it out into a crate: https://docs.rs/memory_unitsDon't think we want to use exactly that, but more that kind of thing.
Don't think we want to use exactly that, but more that kind of thing.
I agree. That's inside the code base, but even from a CLI perspective I found it useful to use
bytesize
in thempk.rs
example to more easily just pass in strings like "4KiB" or whatever instead of having to do mental math.
alexcrichton merged PR #8616.
Last updated: Dec 23 2024 at 12:05 UTC