lostman opened Issue #1785:
The documentation for
fn new_memory( &self, ty: MemoryType, reserved_size: Option<u64>, guard_size: u64 ) -> Result<Box<dyn LinearMemory>, String>
states:
The reserved_size value indicates the expected size of the reservation that is to be made for this memory. If this value is None than the implementation is free to allocate memory as it sees fit. If the value is Some, however, then the implementation is expected to reserve that many bytes for the memory's allocation, plus the guard size at the end
Aside: please also note the typo
than
.This sounds like the intended unit for
reserved_size
is bytes which is also supported by itsu64
type . However, the implementation passes in the size in pages:impl RuntimeMemoryCreator for MemoryCreatorProxy { fn new_memory(&self, plan: &MemoryPlan) -> Result<Box<dyn RuntimeLinearMemory>, String> { let ty = MemoryType::new(Limits::new(plan.memory.minimum, plan.memory.maximum)); let reserved_size = match plan.style { MemoryStyle::Static { bound } => Some(bound.into()), MemoryStyle::Dynamic => None, }; self.mem_creator .new_memory(ty, reserved_size, plan.offset_guard_size) .map(|mem| Box::new(LinearMemoryProxy { mem }) as Box<dyn RuntimeLinearMemory>) } }
where:
pub enum MemoryStyle { /// The actual memory can be resized and moved. Dynamic, /// Addresss space is allocated up front. Static { /// The number of mapped and unmapped pages. bound: u32, }, }
I'll be honest, when working with WebAssembly I am often confused as to whether the memory size is expressed in Wasm pages, OS pages, or bytes. Often it is all three at different points in time! Thus, I tend to make my variable names very explicit. May I suggest changing the function signature to:
fn new_memory( &self, ty: MemoryType, reserved_size_in_bytes: Option<u64>, guard_size_in_bytes: u64 ) -> Result<Box<dyn LinearMemory>, String>
?
Or perhaps
reserved_size_in_wasm_pages
if that is the desired unit?
lostman labeled Issue #1785:
The documentation for
fn new_memory( &self, ty: MemoryType, reserved_size: Option<u64>, guard_size: u64 ) -> Result<Box<dyn LinearMemory>, String>
states:
The reserved_size value indicates the expected size of the reservation that is to be made for this memory. If this value is None than the implementation is free to allocate memory as it sees fit. If the value is Some, however, then the implementation is expected to reserve that many bytes for the memory's allocation, plus the guard size at the end
Aside: please also note the typo
than
.This sounds like the intended unit for
reserved_size
is bytes which is also supported by itsu64
type . However, the implementation passes in the size in pages:impl RuntimeMemoryCreator for MemoryCreatorProxy { fn new_memory(&self, plan: &MemoryPlan) -> Result<Box<dyn RuntimeLinearMemory>, String> { let ty = MemoryType::new(Limits::new(plan.memory.minimum, plan.memory.maximum)); let reserved_size = match plan.style { MemoryStyle::Static { bound } => Some(bound.into()), MemoryStyle::Dynamic => None, }; self.mem_creator .new_memory(ty, reserved_size, plan.offset_guard_size) .map(|mem| Box::new(LinearMemoryProxy { mem }) as Box<dyn RuntimeLinearMemory>) } }
where:
pub enum MemoryStyle { /// The actual memory can be resized and moved. Dynamic, /// Addresss space is allocated up front. Static { /// The number of mapped and unmapped pages. bound: u32, }, }
I'll be honest, when working with WebAssembly I am often confused as to whether the memory size is expressed in Wasm pages, OS pages, or bytes. Often it is all three at different points in time! Thus, I tend to make my variable names very explicit. May I suggest changing the function signature to:
fn new_memory( &self, ty: MemoryType, reserved_size_in_bytes: Option<u64>, guard_size_in_bytes: u64 ) -> Result<Box<dyn LinearMemory>, String>
?
Or perhaps
reserved_size_in_wasm_pages
if that is the desired unit?
alexcrichton commented on Issue #1785:
The intention here is that the values are all in bytes, so if it's in wasm pages that's definitely a bug! Renaming the parameter names sounds great too!
Would you be up for a PR to update this?
alexcrichton labeled Issue #1785:
The documentation for
fn new_memory( &self, ty: MemoryType, reserved_size: Option<u64>, guard_size: u64 ) -> Result<Box<dyn LinearMemory>, String>
states:
The reserved_size value indicates the expected size of the reservation that is to be made for this memory. If this value is None than the implementation is free to allocate memory as it sees fit. If the value is Some, however, then the implementation is expected to reserve that many bytes for the memory's allocation, plus the guard size at the end
Aside: please also note the typo
than
.This sounds like the intended unit for
reserved_size
is bytes which is also supported by itsu64
type . However, the implementation passes in the size in pages:impl RuntimeMemoryCreator for MemoryCreatorProxy { fn new_memory(&self, plan: &MemoryPlan) -> Result<Box<dyn RuntimeLinearMemory>, String> { let ty = MemoryType::new(Limits::new(plan.memory.minimum, plan.memory.maximum)); let reserved_size = match plan.style { MemoryStyle::Static { bound } => Some(bound.into()), MemoryStyle::Dynamic => None, }; self.mem_creator .new_memory(ty, reserved_size, plan.offset_guard_size) .map(|mem| Box::new(LinearMemoryProxy { mem }) as Box<dyn RuntimeLinearMemory>) } }
where:
pub enum MemoryStyle { /// The actual memory can be resized and moved. Dynamic, /// Addresss space is allocated up front. Static { /// The number of mapped and unmapped pages. bound: u32, }, }
I'll be honest, when working with WebAssembly I am often confused as to whether the memory size is expressed in Wasm pages, OS pages, or bytes. Often it is all three at different points in time! Thus, I tend to make my variable names very explicit. May I suggest changing the function signature to:
fn new_memory( &self, ty: MemoryType, reserved_size_in_bytes: Option<u64>, guard_size_in_bytes: u64 ) -> Result<Box<dyn LinearMemory>, String>
?
Or perhaps
reserved_size_in_wasm_pages
if that is the desired unit?
github-actions[bot] commented on Issue #1785:
Subscribe to Label Action
cc @peterhuene
<details>
This issue or pull request has been labeled: "wasmtime:api"Thus the following users have been cc'd because of the following labels:
- peterhuene: wasmtime:api
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
lostman commented on Issue #1785:
Sure, will make a PR.
lostman closed Issue #1785:
The documentation for
fn new_memory( &self, ty: MemoryType, reserved_size: Option<u64>, guard_size: u64 ) -> Result<Box<dyn LinearMemory>, String>
states:
The reserved_size value indicates the expected size of the reservation that is to be made for this memory. If this value is None than the implementation is free to allocate memory as it sees fit. If the value is Some, however, then the implementation is expected to reserve that many bytes for the memory's allocation, plus the guard size at the end
Aside: please also note the typo
than
.This sounds like the intended unit for
reserved_size
is bytes which is also supported by itsu64
type . However, the implementation passes in the size in pages:impl RuntimeMemoryCreator for MemoryCreatorProxy { fn new_memory(&self, plan: &MemoryPlan) -> Result<Box<dyn RuntimeLinearMemory>, String> { let ty = MemoryType::new(Limits::new(plan.memory.minimum, plan.memory.maximum)); let reserved_size = match plan.style { MemoryStyle::Static { bound } => Some(bound.into()), MemoryStyle::Dynamic => None, }; self.mem_creator .new_memory(ty, reserved_size, plan.offset_guard_size) .map(|mem| Box::new(LinearMemoryProxy { mem }) as Box<dyn RuntimeLinearMemory>) } }
where:
pub enum MemoryStyle { /// The actual memory can be resized and moved. Dynamic, /// Addresss space is allocated up front. Static { /// The number of mapped and unmapped pages. bound: u32, }, }
I'll be honest, when working with WebAssembly I am often confused as to whether the memory size is expressed in Wasm pages, OS pages, or bytes. Often it is all three at different points in time! Thus, I tend to make my variable names very explicit. May I suggest changing the function signature to:
fn new_memory( &self, ty: MemoryType, reserved_size_in_bytes: Option<u64>, guard_size_in_bytes: u64 ) -> Result<Box<dyn LinearMemory>, String>
?
Or perhaps
reserved_size_in_wasm_pages
if that is the desired unit?
Last updated: Dec 23 2024 at 12:05 UTC