Skip to content

Commit

Permalink
Remove the wasmtime_environ::MemoryPlan type
Browse files Browse the repository at this point in the history
This is the equivalent of bytecodealliance#9530 for memories. The goal of this commit is
to eventually remove the abstraction layer of `MemoryPlan` and
`MemoryStyle` in favor of directly reading the configuration of
`Tunables`. The prediction is that it will be simpler to work directly
with configured values instead of a layer of abstraction between the
configuration and the runtime which needs to be evolved independently to
capture how to interpret the configuration.

Like with bytecodealliance#9530 my plan is to eventually remove the `MemoryStyle` type
itself, but that'll be a larger change, so it's deferred to a future
PR.
  • Loading branch information
alexcrichton committed Oct 31, 2024
1 parent 7a49e44 commit 6bffc67
Show file tree
Hide file tree
Showing 23 changed files with 392 additions and 437 deletions.
302 changes: 142 additions & 160 deletions crates/cranelift/src/func_environ.rs

Large diffs are not rendered by default.

29 changes: 10 additions & 19 deletions crates/environ/src/compile/module_environ.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::module::{
FuncRefIndex, Initializer, MemoryInitialization, MemoryInitializer, MemoryPlan, Module,
TableSegment, TableSegmentElements,
FuncRefIndex, Initializer, MemoryInitialization, MemoryInitializer, Module, TableSegment,
TableSegmentElements,
};
use crate::prelude::*;
use crate::{
Expand Down Expand Up @@ -373,12 +373,11 @@ impl<'a, 'data> ModuleEnvironment<'a, 'data> {
self.validator.memory_section(&memories)?;

let cnt = usize::try_from(memories.count()).unwrap();
self.result.module.memory_plans.reserve_exact(cnt);
self.result.module.memories.reserve_exact(cnt);

for entry in memories {
let memory = entry?;
let plan = MemoryPlan::for_memory(memory.into(), &self.tunables);
self.result.module.memory_plans.push(plan);
self.result.module.memories.push(memory.into());
}
}

Expand Down Expand Up @@ -767,10 +766,7 @@ and for re-adding support for interface types you can see this issue:
func_index
}),
EntityType::Table(ty) => EntityIndex::Table(self.result.module.tables.push(ty)),
EntityType::Memory(ty) => {
let plan = MemoryPlan::for_memory(ty, &self.tunables);
EntityIndex::Memory(self.result.module.memory_plans.push(plan))
}
EntityType::Memory(ty) => EntityIndex::Memory(self.result.module.memories.push(ty)),
EntityType::Global(ty) => EntityIndex::Global(self.result.module.globals.push(ty)),
EntityType::Tag(_) => unimplemented!(),
}
Expand Down Expand Up @@ -920,8 +916,8 @@ impl ModuleTranslation<'_> {
// wasm module.
segments: Vec<(usize, StaticMemoryInitializer)>,
}
let mut info = PrimaryMap::with_capacity(self.module.memory_plans.len());
for _ in 0..self.module.memory_plans.len() {
let mut info = PrimaryMap::with_capacity(self.module.memories.len());
for _ in 0..self.module.memories.len() {
info.push(Memory {
data_size: 0,
min_addr: u64::MAX,
Expand All @@ -940,16 +936,11 @@ impl ModuleTranslation<'_> {
&mut self,
memory_index: MemoryIndex,
) -> Result<u64, SizeOverflow> {
self.module.memory_plans[memory_index]
.memory
.minimum_byte_size()
self.module.memories[memory_index].minimum_byte_size()
}

fn eval_offset(&mut self, memory_index: MemoryIndex, expr: &ConstExpr) -> Option<u64> {
match (
expr.ops(),
self.module.memory_plans[memory_index].memory.idx_type,
) {
match (expr.ops(), self.module.memories[memory_index].idx_type) {
(&[ConstOp::I32Const(offset)], IndexType::I32) => {
Some(offset.unsigned().into())
}
Expand Down Expand Up @@ -1002,7 +993,7 @@ impl ModuleTranslation<'_> {
// initializer can be created. This can be handled technically but
// would require some more changes to help fix the assert elsewhere
// that this protects against.
if self.module.memory_plans[i].memory.page_size() < page_size {
if self.module.memories[i].page_size() < page_size {
return;
}

Expand Down
4 changes: 2 additions & 2 deletions crates/environ/src/component/translate/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -966,8 +966,8 @@ impl<'a> Inliner<'a> {
Some(memory) => match &self.runtime_instances[memory.instance] {
InstanceModule::Static(idx) => match &memory.item {
ExportItem::Index(i) => {
let plan = &self.nested_modules[*idx].module.memory_plans[*i];
match plan.memory.idx_type {
let ty = &self.nested_modules[*idx].module.memories[*i];
match ty.idx_type {
IndexType::I32 => false,
IndexType::I64 => true,
}
Expand Down
47 changes: 11 additions & 36 deletions crates/environ/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,37 +72,6 @@ impl MemoryStyle {
}
}

/// A WebAssembly linear memory description along with our chosen style for
/// implementing it.
#[derive(Debug, Clone, Hash, Serialize, Deserialize)]
pub struct MemoryPlan {
/// The WebAssembly linear memory description.
pub memory: Memory,
/// Our chosen implementation style.
pub style: MemoryStyle,
/// Chosen size of a guard page before the linear memory allocation.
pub pre_guard_size: u64,
/// Our chosen offset-guard size.
pub offset_guard_size: u64,
}

impl MemoryPlan {
/// Draw up a plan for implementing a `Memory`.
pub fn for_memory(memory: Memory, tunables: &Tunables) -> Self {
let (style, offset_guard_size) = MemoryStyle::for_memory(memory, tunables);
Self {
memory,
style,
offset_guard_size,
pre_guard_size: if tunables.guard_before_linear_memory {
offset_guard_size
} else {
0
},
}
}
}

/// A WebAssembly linear memory initializer.
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct MemoryInitializer {
Expand Down Expand Up @@ -466,7 +435,7 @@ pub struct Module {
pub tables: PrimaryMap<TableIndex, Table>,

/// WebAssembly linear memory plans.
pub memory_plans: PrimaryMap<MemoryIndex, MemoryPlan>,
pub memories: PrimaryMap<MemoryIndex, Memory>,

/// WebAssembly global variables.
pub globals: PrimaryMap<GlobalIndex, Global>,
Expand Down Expand Up @@ -571,19 +540,19 @@ impl Module {
#[inline]
pub fn owned_memory_index(&self, memory: DefinedMemoryIndex) -> OwnedMemoryIndex {
assert!(
memory.index() < self.memory_plans.len(),
memory.index() < self.memories.len(),
"non-shared memory must have an owned index"
);

// Once we know that the memory index is not greater than the number of
// plans, we can iterate through the plans up to the memory index and
// count how many are not shared (i.e., owned).
let owned_memory_index = self
.memory_plans
.memories
.iter()
.skip(self.num_imported_memories)
.take(memory.index())
.filter(|(_, mp)| !mp.memory.shared)
.filter(|(_, mp)| !mp.shared)
.count();
OwnedMemoryIndex::new(owned_memory_index)
}
Expand Down Expand Up @@ -634,7 +603,7 @@ impl Module {
match index {
EntityIndex::Global(i) => EntityType::Global(self.globals[i]),
EntityIndex::Table(i) => EntityType::Table(self.tables[i]),
EntityIndex::Memory(i) => EntityType::Memory(self.memory_plans[i].memory),
EntityIndex::Memory(i) => EntityType::Memory(self.memories[i]),
EntityIndex::Function(i) => {
EntityType::Function(EngineOrModuleTypeIndex::Module(self.functions[i].signature))
}
Expand Down Expand Up @@ -662,6 +631,12 @@ impl Module {
pub fn num_defined_tables(&self) -> usize {
self.tables.len() - self.num_imported_tables
}

/// Returns the number of memories defined by this module itself: all
/// memories minus imported memories.
pub fn num_defined_memories(&self) -> usize {
self.memories.len() - self.num_imported_memories
}
}

/// Type information about functions in a wasm module.
Expand Down
8 changes: 3 additions & 5 deletions crates/environ/src/vmoffsets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,10 +338,10 @@ impl<P: PtrSize> VMOffsets<P> {
/// Return a new `VMOffsets` instance, for a given pointer size.
pub fn new(ptr: P, module: &Module) -> Self {
let num_owned_memories = module
.memory_plans
.memories
.iter()
.skip(module.num_imported_memories)
.filter(|p| !p.1.memory.shared)
.filter(|p| !p.1.shared)
.count()
.try_into()
.unwrap();
Expand All @@ -352,9 +352,7 @@ impl<P: PtrSize> VMOffsets<P> {
num_imported_memories: cast_to_u32(module.num_imported_memories),
num_imported_globals: cast_to_u32(module.num_imported_globals),
num_defined_tables: cast_to_u32(module.num_defined_tables()),
num_defined_memories: cast_to_u32(
module.memory_plans.len() - module.num_imported_memories,
),
num_defined_memories: cast_to_u32(module.num_defined_memories()),
num_owned_memories,
num_defined_globals: cast_to_u32(module.globals.len() - module.num_imported_globals),
num_escaped_funcs: cast_to_u32(module.num_escaped_funcs),
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmtime/src/runtime/externals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ impl Extern {
Extern::Func(Func::from_wasmtime_function(f, store))
}
crate::runtime::vm::Export::Memory(m) => {
if m.memory.memory.shared {
if m.memory.shared {
Extern::SharedMemory(SharedMemory::from_wasmtime_memory(m, store))
} else {
Extern::Memory(Memory::from_wasmtime_memory(m, store))
Expand Down
24 changes: 14 additions & 10 deletions crates/wasmtime/src/runtime/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use core::fmt;
use core::ops::Range;
use core::slice;
use core::time::Duration;
use wasmtime_environ::MemoryPlan;

pub use crate::runtime::vm::WaitResult;

Expand Down Expand Up @@ -294,7 +293,7 @@ impl Memory {
/// ```
pub fn ty(&self, store: impl AsContext) -> MemoryType {
let store = store.as_context();
let ty = &store[self.0].memory.memory;
let ty = &store[self.0].memory;
MemoryType::from_wasmtime_memory(&ty)
}

Expand Down Expand Up @@ -499,7 +498,7 @@ impl Memory {
}

pub(crate) fn _page_size(&self, store: &StoreOpaque) -> u64 {
store[self.0].memory.memory.page_size()
store[self.0].memory.page_size()
}

/// Returns the log2 of this memory's page size, in bytes.
Expand All @@ -519,7 +518,7 @@ impl Memory {
}

pub(crate) fn _page_size_log2(&self, store: &StoreOpaque) -> u8 {
store[self.0].memory.memory.page_size_log2
store[self.0].memory.page_size_log2
}

/// Grows this WebAssembly memory by `delta` pages.
Expand Down Expand Up @@ -632,7 +631,7 @@ impl Memory {
}

pub(crate) fn wasmtime_ty<'a>(&self, store: &'a StoreData) -> &'a wasmtime_environ::Memory {
&store[self.0].memory.memory
&store[self.0].memory
}

pub(crate) fn vmimport(&self, store: &StoreOpaque) -> crate::runtime::vm::VMMemoryImport {
Expand Down Expand Up @@ -808,9 +807,9 @@ impl SharedMemory {
debug_assert!(ty.maximum().is_some());

let tunables = engine.tunables();
let plan = MemoryPlan::for_memory(*ty.wasmtime_memory(), tunables);
let page_size_log2 = plan.memory.page_size_log2;
let memory = crate::runtime::vm::SharedMemory::new(plan)?;
let ty = ty.wasmtime_memory();
let page_size_log2 = ty.page_size_log2;
let memory = crate::runtime::vm::SharedMemory::new(ty, tunables)?;

Ok(Self {
vm: memory,
Expand Down Expand Up @@ -1055,8 +1054,13 @@ mod tests {
let ty = MemoryType::new(1, None);
let mem = Memory::new(&mut store, ty).unwrap();
let store = store.as_context();
assert_eq!(store[mem.0].memory.offset_guard_size, 0);
match &store[mem.0].memory.style {
let (style, offset_guard_size) = wasmtime_environ::MemoryStyle::for_memory(
store[mem.0].memory,
store.engine().tunables(),
);

assert_eq!(offset_guard_size, 0);
match style {
wasmtime_environ::MemoryStyle::Dynamic { .. } => {}
other => panic!("unexpected style {other:?}"),
}
Expand Down
6 changes: 3 additions & 3 deletions crates/wasmtime/src/runtime/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -876,12 +876,12 @@ impl Module {
/// ```
pub fn resources_required(&self) -> ResourcesRequired {
let em = self.env_module();
let num_memories = u32::try_from(em.memory_plans.len() - em.num_imported_memories).unwrap();
let num_memories = u32::try_from(em.num_defined_memories()).unwrap();
let max_initial_memory_size = em
.memory_plans
.memories
.values()
.skip(em.num_imported_memories)
.map(|plan| plan.memory.limits.min)
.map(|memory| memory.limits.min)
.max();
let num_tables = u32::try_from(em.num_defined_tables()).unwrap();
let max_initial_table_size = em
Expand Down
2 changes: 1 addition & 1 deletion crates/wasmtime/src/runtime/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1293,7 +1293,7 @@ impl StoreOpaque {
}

let module = module.env_module();
let memories = module.memory_plans.len() - module.num_imported_memories;
let memories = module.num_defined_memories();
let tables = module.num_defined_tables();

bump(&mut self.instance_count, self.instance_limit, 1, "instance")?;
Expand Down
Loading

0 comments on commit 6bffc67

Please sign in to comment.