Optimize table.init instruction and instantiation (#2847)

* Optimize `table.init` instruction and instantiation

This commit optimizes table initialization as part of instance
instantiation and also applies the same optimization to the `table.init`
instruction. One part of this commit is to remove some preexisting
duplication between instance instantiation and the `table.init`
instruction itself, after this the actual implementation of `table.init`
is optimized to effectively have fewer bounds checks in fewer places and
have a much tighter loop for instantiation.

A big fallout from this change is that memory/table initializer offsets
are now stored as `u32` instead of `usize` to remove a few casts in a
few places. This ended up requiring moving some overflow checks that
happened in parsing to later in code itself because otherwise the wrong
spec test errors are emitted during testing. I've tried to trace where
these can possibly overflow but I think that I managed to get
everything.

In a local synthetic test where an empty module with a single 80,000
element initializer this improves total instantiation time by 4x (562us
=> 141us)

* Review comments
This commit is contained in:
Alex Crichton
2021-04-19 18:44:48 -05:00
committed by GitHub
parent 2864bb4a0f
commit 193551a8d6
8 changed files with 137 additions and 126 deletions

View File

@@ -573,11 +573,11 @@ impl Instance {
return None;
}
Some(unsafe { &*self.anyfunc_ptr(index) })
unsafe { Some(&*self.vmctx_plus_offset(self.offsets.vmctx_anyfunc(index))) }
}
unsafe fn anyfunc_ptr(&self, index: FuncIndex) -> *mut VMCallerCheckedAnyfunc {
self.vmctx_plus_offset(self.offsets.vmctx_anyfunc(index))
unsafe fn anyfunc_base(&self) -> *mut VMCallerCheckedAnyfunc {
self.vmctx_plus_offset(self.offsets.vmctx_anyfuncs_begin())
}
fn find_passive_segment<'a, I, D, T>(
@@ -611,38 +611,56 @@ impl Instance {
src: u32,
len: u32,
) -> Result<(), Trap> {
// https://webassembly.github.io/bulk-memory-operations/core/exec/instructions.html#exec-table-init
let table = self.get_table(table_index);
let elements = Self::find_passive_segment(
elem_index,
&self.module.passive_elements_map,
&self.module.passive_elements,
&self.dropped_elements,
);
self.table_init_segment(table_index, elements, dst, src, len)
}
if src
.checked_add(len)
.map_or(true, |n| n as usize > elements.len())
|| dst.checked_add(len).map_or(true, |m| m > table.size())
pub(crate) fn table_init_segment(
&self,
table_index: TableIndex,
elements: &[FuncIndex],
dst: u32,
src: u32,
len: u32,
) -> Result<(), Trap> {
// https://webassembly.github.io/bulk-memory-operations/core/exec/instructions.html#exec-table-init
let table = self.get_table(table_index);
let elements = match elements
.get(usize::try_from(src).unwrap()..)
.and_then(|s| s.get(..usize::try_from(len).unwrap()))
{
return Err(Trap::wasm(ir::TrapCode::TableOutOfBounds));
Some(elements) => elements,
None => return Err(Trap::wasm(ir::TrapCode::TableOutOfBounds)),
};
match table.element_type() {
TableElementType::Func => unsafe {
let base = self.anyfunc_base();
table.init_funcs(
dst,
elements.iter().map(|idx| {
if *idx == FuncIndex::reserved_value() {
ptr::null_mut()
} else {
debug_assert!(idx.as_u32() < self.offsets.num_defined_functions);
base.add(usize::try_from(idx.as_u32()).unwrap())
}
}),
)?;
},
TableElementType::Val(_) => {
debug_assert!(elements.iter().all(|e| *e == FuncIndex::reserved_value()));
table.fill(dst, TableElement::ExternRef(None), len)?;
}
}
// TODO(#983): investigate replacing this get/set loop with a `memcpy`.
for (dst, src) in (dst..dst + len).zip(src..src + len) {
let elem = self
.get_caller_checked_anyfunc(elements[src as usize])
.map_or(ptr::null_mut(), |f: &VMCallerCheckedAnyfunc| {
f as *const VMCallerCheckedAnyfunc as *mut _
});
table
.set(dst, TableElement::FuncRef(elem))
.expect("should never panic because we already did the bounds check above");
}
Ok(())
}
@@ -773,16 +791,26 @@ impl Instance {
src: u32,
len: u32,
) -> Result<(), Trap> {
// https://webassembly.github.io/bulk-memory-operations/core/exec/instructions.html#exec-memory-init
let memory = self.get_memory(memory_index);
let data = Self::find_passive_segment(
data_index,
&self.module.passive_data_map,
&self.module.passive_data,
&self.dropped_data,
);
self.memory_init_segment(memory_index, &data, dst, src, len)
}
pub(crate) fn memory_init_segment(
&self,
memory_index: MemoryIndex,
data: &[u8],
dst: u32,
src: u32,
len: u32,
) -> Result<(), Trap> {
// https://webassembly.github.io/bulk-memory-operations/core/exec/instructions.html#exec-memory-init
let memory = self.get_memory(memory_index);
if src
.checked_add(len)

View File

@@ -2,7 +2,7 @@ use crate::externref::{ModuleInfoLookup, VMExternRefActivationsTable, EMPTY_MODU
use crate::imports::Imports;
use crate::instance::{Instance, InstanceHandle, ResourceLimiter, RuntimeMemoryCreator};
use crate::memory::{DefaultMemoryCreator, Memory};
use crate::table::{Table, TableElement};
use crate::table::Table;
use crate::traphandlers::Trap;
use crate::vmcontext::{
VMBuiltinFunctionsArray, VMCallerCheckedAnyfunc, VMContext, VMFunctionBody, VMFunctionImport,
@@ -19,10 +19,9 @@ use std::rc::Rc;
use std::slice;
use std::sync::Arc;
use thiserror::Error;
use wasmtime_environ::entity::{packed_option::ReservedValue, EntityRef, EntitySet, PrimaryMap};
use wasmtime_environ::entity::{EntityRef, EntitySet, PrimaryMap};
use wasmtime_environ::wasm::{
DefinedFuncIndex, DefinedMemoryIndex, DefinedTableIndex, FuncIndex, GlobalInit, SignatureIndex,
TableElementType, WasmType,
DefinedFuncIndex, DefinedMemoryIndex, DefinedTableIndex, GlobalInit, SignatureIndex, WasmType,
};
use wasmtime_environ::{
ir, MemoryInitialization, MemoryInitializer, Module, ModuleType, TableInitializer, VMOffsets,
@@ -212,7 +211,7 @@ impl<'a> From<&'a PrimaryMap<SignatureIndex, VMSharedSignatureIndex>> for Shared
fn get_table_init_start(
init: &TableInitializer,
instance: &Instance,
) -> Result<usize, InstantiationError> {
) -> Result<u32, InstantiationError> {
match init.base {
Some(base) => {
let val = unsafe {
@@ -223,7 +222,7 @@ fn get_table_init_start(
}
};
init.offset.checked_add(val as usize).ok_or_else(|| {
init.offset.checked_add(val).ok_or_else(|| {
InstantiationError::Link(LinkError(
"element segment global base overflows".to_owned(),
))
@@ -237,6 +236,7 @@ fn check_table_init_bounds(instance: &Instance) -> Result<(), InstantiationError
for init in &instance.module.table_initializers {
let table = instance.get_table(init.table_index);
let start = get_table_init_start(init, instance)?;
let start = usize::try_from(start).unwrap();
let end = start.checked_add(init.elements.len());
match end {
@@ -256,34 +256,15 @@ fn check_table_init_bounds(instance: &Instance) -> Result<(), InstantiationError
fn initialize_tables(instance: &Instance) -> Result<(), InstantiationError> {
for init in &instance.module.table_initializers {
let table = instance.get_table(init.table_index);
let start = get_table_init_start(init, instance)?;
let end = start.checked_add(init.elements.len());
match end {
Some(end) if end <= table.size() as usize => {
for (i, func_idx) in init.elements.iter().enumerate() {
let item = match table.element_type() {
TableElementType::Func => instance
.get_caller_checked_anyfunc(*func_idx)
.map_or(ptr::null_mut(), |f: &VMCallerCheckedAnyfunc| {
f as *const VMCallerCheckedAnyfunc as *mut VMCallerCheckedAnyfunc
})
.into(),
TableElementType::Val(_) => {
assert!(*func_idx == FuncIndex::reserved_value());
TableElement::ExternRef(None)
}
};
table.set(u32::try_from(start + i).unwrap(), item).unwrap();
}
}
_ => {
return Err(InstantiationError::Trap(Trap::wasm(
ir::TrapCode::TableOutOfBounds,
)))
}
}
instance
.table_init_segment(
init.table_index,
&init.elements,
get_table_init_start(init, instance)?,
0,
init.elements.len() as u32,
)
.map_err(InstantiationError::Trap)?;
}
Ok(())
@@ -292,7 +273,7 @@ fn initialize_tables(instance: &Instance) -> Result<(), InstantiationError> {
fn get_memory_init_start(
init: &MemoryInitializer,
instance: &Instance,
) -> Result<usize, InstantiationError> {
) -> Result<u32, InstantiationError> {
match init.base {
Some(base) => {
let val = unsafe {
@@ -303,7 +284,7 @@ fn get_memory_init_start(
}
};
init.offset.checked_add(val as usize).ok_or_else(|| {
init.offset.checked_add(val).ok_or_else(|| {
InstantiationError::Link(LinkError("data segment global base overflows".to_owned()))
})
}
@@ -311,24 +292,6 @@ fn get_memory_init_start(
}
}
unsafe fn get_memory_slice<'instance>(
init: &MemoryInitializer,
instance: &'instance Instance,
) -> &'instance mut [u8] {
let memory = if let Some(defined_memory_index) =
instance.module.defined_memory_index(init.memory_index)
{
instance.memory(defined_memory_index)
} else {
let import = instance.imported_memory(init.memory_index);
let foreign_instance = (&mut *(import).vmctx).instance();
let foreign_memory = &mut *(import).from;
let foreign_index = foreign_instance.memory_index(foreign_memory);
foreign_instance.memory(foreign_index)
};
&mut *ptr::slice_from_raw_parts_mut(memory.base, memory.current_length)
}
fn check_memory_init_bounds(
instance: &Instance,
initializers: &[MemoryInitializer],
@@ -336,6 +299,7 @@ fn check_memory_init_bounds(
for init in initializers {
let memory = instance.get_memory(init.memory_index);
let start = get_memory_init_start(init, instance)?;
let start = usize::try_from(start).unwrap();
let end = start.checked_add(init.data.len());
match end {
@@ -358,21 +322,15 @@ fn initialize_memories(
initializers: &[MemoryInitializer],
) -> Result<(), InstantiationError> {
for init in initializers {
let memory = instance.get_memory(init.memory_index);
let start = get_memory_init_start(init, instance)?;
let end = start.checked_add(init.data.len());
match end {
Some(end) if end <= memory.current_length => {
let mem_slice = unsafe { get_memory_slice(init, instance) };
mem_slice[start..end].copy_from_slice(&init.data);
}
_ => {
return Err(InstantiationError::Trap(Trap::wasm(
ir::TrapCode::HeapOutOfBounds,
)))
}
}
instance
.memory_init_segment(
init.memory_index,
&init.data,
get_memory_init_start(init, instance)?,
0,
init.data.len() as u32,
)
.map_err(InstantiationError::Trap)?;
}
Ok(())
@@ -496,6 +454,7 @@ unsafe fn initialize_vmcontext(instance: &Instance, req: InstanceAllocationReque
);
// Initialize the functions
let mut base = instance.anyfunc_base();
for (index, sig) in instance.module.functions.iter() {
let type_index = req.shared_signatures.lookup(*sig);
@@ -510,13 +469,14 @@ unsafe fn initialize_vmcontext(instance: &Instance, req: InstanceAllocationReque
};
ptr::write(
instance.anyfunc_ptr(index),
base,
VMCallerCheckedAnyfunc {
func_ptr,
type_index,
vmctx,
},
);
base = base.add(1);
}
// Initialize the defined tables

View File

@@ -7,7 +7,7 @@ use crate::{ResourceLimiter, Trap, VMExternRef};
use anyhow::{bail, Result};
use std::cell::{Cell, RefCell};
use std::cmp::min;
use std::convert::TryInto;
use std::convert::{TryFrom, TryInto};
use std::ops::Range;
use std::ptr;
use std::rc::Rc;
@@ -212,6 +212,32 @@ impl Table {
}
}
/// Fill `table[dst..]` with values from `items`
///
/// Returns a trap error on out-of-bounds accesses.
pub fn init_funcs(
&self,
dst: u32,
items: impl ExactSizeIterator<Item = *mut VMCallerCheckedAnyfunc>,
) -> Result<(), Trap> {
assert!(self.element_type() == TableElementType::Func);
self.with_elements_mut(|elements| {
let elements = match elements
.get_mut(usize::try_from(dst).unwrap()..)
.and_then(|s| s.get_mut(..items.len()))
{
Some(elements) => elements,
None => return Err(Trap::wasm(ir::TrapCode::TableOutOfBounds)),
};
for (item, slot) in items.zip(elements) {
*slot = item as *mut u8;
}
Ok(())
})
}
/// Fill `table[dst..dst + len]` with `val`.
///
/// Returns a trap error on out-of-bounds accesses.