Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Instance management within the CallStack (take 2) #1065

Merged
merged 58 commits into from
Jun 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
3f90b4a
initial WIP work
Robbepop Jun 3, 2024
a5be772
Merge branch 'master' into rf-refactor-call-stack
Robbepop Jun 4, 2024
7e2d476
refactor InstanceStack::push
Robbepop Jun 4, 2024
23d942a
Merge branch 'master' into rf-refactor-call-stack
Robbepop Jun 8, 2024
972f954
fix after merge
Robbepop Jun 8, 2024
f28e61f
add TopVec type
Robbepop Jun 10, 2024
ac2d141
add missing import
Robbepop Jun 10, 2024
57845dd
use TopVec in InstanceStack
Robbepop Jun 10, 2024
7507bba
add InstanceStack::reset
Robbepop Jun 10, 2024
a00d954
rename IndexedInstance -> InstanceAndHeight
Robbepop Jun 10, 2024
1590b7f
rename field: calls -> frames
Robbepop Jun 10, 2024
4045858
apply clippy suggestion
Robbepop Jun 10, 2024
c5e45c1
improve docs
Robbepop Jun 10, 2024
6b8064f
rename index -> height
Robbepop Jun 10, 2024
29a724b
return new Instance in CallStack::pop
Robbepop Jun 10, 2024
8e1aa6f
remove unused TopVec methods
Robbepop Jun 10, 2024
5860d43
add inline annotations
Robbepop Jun 10, 2024
99f7673
remove no longer needed import
Robbepop Jun 10, 2024
01b5926
improve docs
Robbepop Jun 10, 2024
ffe568e
rename top -> last as in Rust's Vec
Robbepop Jun 10, 2024
38e5477
improve docs
Robbepop Jun 10, 2024
966189e
remove commented out code
Robbepop Jun 10, 2024
b8e25d7
fix typo
Robbepop Jun 10, 2024
6243220
rename TopVec -> HeadVec
Robbepop Jun 10, 2024
f4f2170
refactor CallStack to no longer use InstanceStack
Robbepop Jun 10, 2024
7a8b510
add some inline annotations
Robbepop Jun 10, 2024
3496258
put cached memory bytes directly into Executor
Robbepop Jun 11, 2024
6b372c6
fix doc link and cleanup code
Robbepop Jun 11, 2024
f19bef9
Merge branch 'rf-opt-mem-access' into rf-refactor-call-stack-v2
Robbepop Jun 11, 2024
2141521
fix code from merge
Robbepop Jun 11, 2024
de8b30e
add StackOffsets abstraction
Robbepop Jun 12, 2024
cf5e223
add changed_instance field to CallFrame
Robbepop Jun 12, 2024
765c112
add inline annotations
Robbepop Jun 13, 2024
a9b3e9e
add missing docs to default_memory method
Robbepop Jun 13, 2024
e163f78
add CachedMemory type
Robbepop Jun 13, 2024
87a9bdd
add safety comments
Robbepop Jun 14, 2024
0ba0156
replace InstanceCache with finer grained caches
Robbepop Jun 14, 2024
d45cad4
add hints to global variable executors
Robbepop Jun 14, 2024
1cf06e7
define From for bytecode index type to convert to u32
Robbepop Jun 14, 2024
13ba89b
use macro to generate get_entity funcs
Robbepop Jun 14, 2024
ac48bc3
inline(always) for instance getter
Robbepop Jun 14, 2024
d50fd18
fix doc links
Robbepop Jun 14, 2024
c5b9983
apply clippy suggestions
Robbepop Jun 14, 2024
a7df9e8
apply rustfmt
Robbepop Jun 14, 2024
0c35638
Merge branch 'master' into rf-remove-instance-cache
Robbepop Jun 14, 2024
9467dc3
Merge branch 'master' into rf-remove-instance-cache
Robbepop Jun 14, 2024
375b911
Merge branch 'master' into rf-remove-instance-cache
Robbepop Jun 14, 2024
9d664de
Merge branch 'master' into rf-remove-instance-cache
Robbepop Jun 15, 2024
455ab19
use old docs for memory field
Robbepop Jun 15, 2024
3298131
Merge branch 'master' into rf-remove-instance-cache
Robbepop Jun 15, 2024
9aca7ef
remove invalid code after merge
Robbepop Jun 15, 2024
279c744
remove inline annotations not present before this PR
Robbepop Jun 15, 2024
10c7bdf
replace some #[inline(always)] with #[inline]
Robbepop Jun 15, 2024
8655cad
move HeadVec to wasmi_collections
Robbepop Jun 15, 2024
8c903e4
use Option::replace where applicable
Robbepop Jun 15, 2024
b88b59a
add basic derives to HeadVec
Robbepop Jun 15, 2024
54be939
swap field ordering in InstanceAndHeight
Robbepop Jun 15, 2024
ceade4e
remove InstanceAndHeight due to redundant information
Robbepop Jun 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions crates/collections/src/head_vec.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
use core::mem;
use std::vec::Vec;

/// A [`Vec`]-like data structure with fast access to the last item.
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
pub struct HeadVec<T> {
/// The top (or last) item in the [`HeadVec`].
head: Option<T>,
/// The rest of the items in the [`HeadVec`] excluding the last item.
rest: Vec<T>,
}

impl<T> Default for HeadVec<T> {
#[inline]
fn default() -> Self {
Self {
head: None,
rest: Vec::new(),
}
}
}

impl<T> HeadVec<T> {
/// Removes all items from the [`HeadVec`].
#[inline]
pub fn clear(&mut self) {
self.head = None;
self.rest.clear();
}

/// Returns the number of items stored in the [`HeadVec`].
#[inline]
pub fn len(&self) -> usize {
match self.head {
Some(_) => 1 + self.rest.len(),
None => 0,

Check warning on line 36 in crates/collections/src/head_vec.rs

View check run for this annotation

Codecov / codecov/patch

crates/collections/src/head_vec.rs#L33-L36

Added lines #L33 - L36 were not covered by tests
}
}

/// Returns `true` if the [`HeadVec`] contains no items.
#[inline]
pub fn is_empty(&self) -> bool {
self.len() == 0

Check warning on line 43 in crates/collections/src/head_vec.rs

View check run for this annotation

Codecov / codecov/patch

crates/collections/src/head_vec.rs#L42-L43

Added lines #L42 - L43 were not covered by tests
}

/// Returns a shared reference to the last item in the [`HeadVec`] if any.
///
/// Returns `None` if the [`HeadVec`] is empty.
#[inline]
pub fn last(&self) -> Option<&T> {
self.head.as_ref()
}

/// Returns an exclusive reference to the last item in the [`HeadVec`] if any.
///
/// Returns `None` if the [`HeadVec`] is empty.
#[inline]
pub fn last_mut(&mut self) -> Option<&mut T> {
self.head.as_mut()

Check warning on line 59 in crates/collections/src/head_vec.rs

View check run for this annotation

Codecov / codecov/patch

crates/collections/src/head_vec.rs#L58-L59

Added lines #L58 - L59 were not covered by tests
}

/// Pushes a new `value` onto the [`HeadVec`].
#[inline]
pub fn push(&mut self, value: T) {
let prev_head = self.head.replace(value);
if let Some(prev_head) = prev_head {
self.rest.push(prev_head);
}
}

/// Pops the last `value` from the [`HeadVec`] if any.
#[inline]
pub fn pop(&mut self) -> Option<T> {
let new_top = self.rest.pop();
mem::replace(&mut self.head, new_top)
}
}
2 changes: 2 additions & 0 deletions crates/collections/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ extern crate std;

pub mod arena;
pub mod hash;
mod head_vec;
pub mod map;
pub mod set;
pub mod string_interner;
Expand All @@ -48,6 +49,7 @@ mod tests;
#[doc(inline)]
pub use self::{
arena::{Arena, ComponentVec, DedupArena},
head_vec::HeadVec,
map::Map,
set::Set,
string_interner::StringInterner,
Expand Down
3 changes: 1 addition & 2 deletions crates/wasmi/src/engine/executor/instrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,7 @@ impl<'engine> Executor<'engine> {
#[inline(always)]
fn instance(call_stack: &CallStack) -> &Instance {
call_stack
.peek()
.map(CallFrame::instance)
.instance()
.expect("missing instance for non-empty call stack")
}

Expand Down
31 changes: 19 additions & 12 deletions crates/wasmi/src/engine/executor/instrs/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,6 @@
&mut self,
results: RegisterSpan,
func: &CompiledFuncEntity,
instance: &Instance,
) -> Result<CallFrame, Error> {
// We have to reinstantiate the `self.sp` [`FrameRegisters`] since we just called
// [`ValueStack::alloc_call_frame`] which might invalidate all live [`FrameRegisters`].
Expand All @@ -227,7 +226,7 @@
self.sp = unsafe { this.stack_ptr_at(caller.base_offset()) };
})?;
let instr_ptr = InstructionPtr::new(func.instrs().as_ptr());
let frame = CallFrame::new(instr_ptr, offsets, results, *instance);
let frame = CallFrame::new(instr_ptr, offsets, results);
if <C as CallContext>::HAS_PARAMS {
self.copy_call_params(&mut uninit_params);
}
Expand Down Expand Up @@ -300,8 +299,7 @@
mut instance: Option<Instance>,
) -> Result<(), Error> {
let func = self.code_map.get(Some(store.fuel_mut()), func)?;
let instance = instance.get_or_insert_with(|| *Self::instance(self.call_stack));
let mut called = self.dispatch_compiled_func::<C>(results, func, instance)?;
let mut called = self.dispatch_compiled_func::<C>(results, func)?;
match <C as CallContext>::KIND {
CallKind::Nested => {
// We need to update the instruction pointer of the caller call frame.
Expand All @@ -318,11 +316,16 @@
// on the value stack which is what the function expects. After this operation we ensure
// that `self.sp` is adjusted via a call to `init_call_frame` since it may have been
// invalidated by this method.
unsafe { Stack::merge_call_frames(self.call_stack, self.value_stack, &mut called) };
let caller_instance = unsafe {
Stack::merge_call_frames(self.call_stack, self.value_stack, &mut called)
};
if let Some(caller_instance) = caller_instance {
instance.get_or_insert(caller_instance);
}
}
}
self.init_call_frame(&called);
self.call_stack.push(called)?;
self.call_stack.push(called, instance)?;
Ok(())
}

Expand Down Expand Up @@ -495,11 +498,15 @@
) -> Result<(), Error> {
let (len_params, len_results) = self.func_types.len_params_results(host_func.ty_dedup());
let max_inout = len_params.max(len_results);
let instance = *self
.call_stack
.instance()
.expect("need to have an instance on the call stack");

Check warning on line 504 in crates/wasmi/src/engine/executor/instrs/call.rs

View check run for this annotation

Codecov / codecov/patch

crates/wasmi/src/engine/executor/instrs/call.rs#L502-L504

Added lines #L502 - L504 were not covered by tests
// We have to reinstantiate the `self.sp` [`FrameRegisters`] since we just called
// [`ValueStack::reserve`] which might invalidate all live [`FrameRegisters`].
let caller = match <C as CallContext>::KIND {
CallKind::Nested => self.call_stack.peek().copied(),
CallKind::Tail => self.call_stack.pop(),
CallKind::Tail => self.call_stack.pop().map(|(frame, _instance)| frame),
}
.expect("need to have a caller on the call stack");
let buffer = self.value_stack.extend_by(max_inout, |this| {
Expand All @@ -513,13 +520,13 @@
if matches!(<C as CallContext>::KIND, CallKind::Nested) {
self.update_instr_ptr_at(1);
}
self.dispatch_host_func::<T>(store, host_func, caller)
self.dispatch_host_func::<T>(store, host_func, &instance)
.map_err(|error| match self.call_stack.is_empty() {
true => error,
false => ResumableHostError::new(error, *func, results).into(),
})?;
self.memory.update(&mut store.inner, caller.instance());
self.global.update(&mut store.inner, caller.instance());
self.memory.update(&mut store.inner, &instance);
self.global.update(&mut store.inner, &instance);
let results = results.iter(len_results);
let returned = self.value_stack.drop_return(max_inout);
for (result, value) in results.zip(returned) {
Expand All @@ -542,14 +549,14 @@
&mut self,
store: &mut Store<T>,
host_func: HostFuncEntity,
caller: CallFrame,
instance: &Instance,
) -> Result<(usize, usize), Error> {
dispatch_host_func(
store,
self.func_types,
self.value_stack,
host_func,
Some(caller.instance()),
Some(instance),
)
}

Expand Down
10 changes: 6 additions & 4 deletions crates/wasmi/src/engine/executor/instrs/return_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,19 @@ impl<'engine> Executor<'engine> {
/// from the returning callee to the caller.
#[inline(always)]
fn return_impl(&mut self, store: &mut StoreInner) -> ReturnOutcome {
let returned = self
let (returned, popped_instance) = self
.call_stack
.pop()
.expect("the executing call frame is always on the stack");
self.value_stack.truncate(returned.frame_offset());
let new_instance = popped_instance.and_then(|_| self.call_stack.instance());
if let Some(new_instance) = new_instance {
self.global.update(store, new_instance);
self.memory.update(store, new_instance);
}
match self.call_stack.peek() {
Some(caller) => {
Self::init_call_frame_impl(self.value_stack, &mut self.sp, &mut self.ip, caller);
let instance = caller.instance();
self.memory.update(store, instance);
self.global.update(store, instance);
ReturnOutcome::Wasm
}
None => ReturnOutcome::Host,
Expand Down
14 changes: 8 additions & 6 deletions crates/wasmi/src/engine/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,12 +215,14 @@
unsafe { uninit_params.init_next(value) };
}
uninit_params.init_zeroes();
self.stack.calls.push(CallFrame::new(
InstructionPtr::new(compiled_func.instrs().as_ptr()),
offsets,
RegisterSpan::new(Register::from_i16(0)),
instance,
))?;
self.stack.calls.push(
CallFrame::new(
InstructionPtr::new(compiled_func.instrs().as_ptr()),
offsets,

Check warning on line 221 in crates/wasmi/src/engine/executor/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/wasmi/src/engine/executor/mod.rs#L221

Added line #L221 was not covered by tests
RegisterSpan::new(Register::from_i16(0)),
),
Some(instance),
)?;
self.execute_func(store)?;
}
FuncEntity::Host(host_func) => {
Expand Down
Loading
Loading