Skip to content

Commit

Permalink
feat: cleanup Engine API
Browse files Browse the repository at this point in the history
1. Hide all wasmtime specific parts.
2. Unexport functions that don't need to be a part of the public API.
3. Rename functions for consistency.
4. Delete some unused code.
  • Loading branch information
Stebalien committed Dec 12, 2023
1 parent dae5922 commit beb08d9
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 80 deletions.
99 changes: 24 additions & 75 deletions fvm/src/engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ mod instance_pool;
use std::any::{Any, TypeId};
use std::collections::hash_map::Entry::{Occupied, Vacant};
use std::collections::HashMap;
use std::ops::Deref;
use std::sync::{Arc, Mutex};

use anyhow::{anyhow, Context};
Expand Down Expand Up @@ -308,14 +307,7 @@ pub struct Engine {
inner: Arc<EngineInner>,
}

impl Deref for Engine {
type Target = wasmtime::Engine;

fn deref(&self) -> &Self::Target {
&self.inner.engine
}
}

/// Release the engine back into the [`EnginePool`].
impl Drop for Engine {
fn drop(&mut self) {
self.inner.concurrency_limit.release();
Expand All @@ -328,29 +320,26 @@ impl Engine {
/// method errors if the code CID is not found in the store.
///
/// Return the original byte code size.
pub fn prepare_actor_code<BS: Blockstore>(
&self,
code_cid: &Cid,
blockstore: BS,
) -> anyhow::Result<usize> {
pub fn preload(&self, code_cid: &Cid, blockstore: &impl Blockstore) -> anyhow::Result<usize> {
let code_cid = self.with_redirect(code_cid);
if let Some(item) = self
match self
.inner
.module_cache
.lock()
.expect("module_cache poisoned")
.get(code_cid)
.entry(*code_cid)
{
return Ok(item.size);
Occupied(e) => Ok(e.get().size),
Vacant(e) => {
let wasm = blockstore.get(code_cid)?.ok_or_else(|| {
anyhow!(
"no wasm bytecode in blockstore for CID {}",
&code_cid.to_string()
)
})?;
Ok(e.insert(self.load_raw(&wasm)?).size)
}
}
let wasm = blockstore.get(code_cid)?.ok_or_else(|| {
anyhow!(
"no wasm bytecode in blockstore for CID {}",
&code_cid.to_string()
)
})?;
// compile and cache instantiated WASM module
self.prepare_wasm_bytecode(code_cid, &wasm)
}

/// Instantiates and caches the Wasm modules for the bytecodes addressed by
Expand All @@ -359,49 +348,31 @@ impl Engine {
/// make this method return an Err immediately.
///
/// Returns the total original byte size of the modules
pub fn preload<'a, BS, I>(&self, blockstore: BS, cids: I) -> anyhow::Result<usize>
where
BS: Blockstore,
I: IntoIterator<Item = &'a Cid>,
{
pub fn preload_all<'a>(
&self,
blockstore: &impl Blockstore,
cids: impl IntoIterator<Item = &'a Cid>,
) -> anyhow::Result<usize> {
let mut total_size = 0usize;
for cid in cids {
log::trace!("preloading code CID {cid}");
let size = self.prepare_actor_code(cid, &blockstore).with_context(|| {
let size = self.preload(cid, &blockstore).with_context(|| {
anyhow!("could not prepare actor with code CID {}", &cid.to_string())
})?;
total_size += size;
}
Ok(total_size)
}

/// Translate the passed CID with a "redirected" CID in case the code has been replaced.
fn with_redirect<'a>(&'a self, k: &'a Cid) -> &'a Cid {
match &self.inner.actor_redirect.get(k) {
Some(cid) => cid,
None => k,
}
}

/// Loads some Wasm code into the engine and prepares it for execution.
pub fn prepare_wasm_bytecode(&self, k: &Cid, wasm: &[u8]) -> anyhow::Result<usize> {
let k = self.with_redirect(k);
let mut cache = self
.inner
.module_cache
.lock()
.expect("module_cache poisoned");
let size = match cache.get(k) {
Some(item) => item.size,
None => {
let m = self.load_raw(wasm)?;
let s = m.size;
cache.insert(*k, m);
s
}
};
Ok(size)
}

/// Load the specified wasm module with the internal Engine instance.
fn load_raw(&self, raw_wasm: &[u8]) -> anyhow::Result<ModuleRecord> {
// First make sure that non-instrumented wasm is valid
Module::validate(&self.inner.engine, raw_wasm)
Expand Down Expand Up @@ -447,7 +418,8 @@ impl Engine {
/// # Safety
///
/// See [`wasmtime::Module::deserialize`] for safety information.
pub unsafe fn load_compiled(&self, k: &Cid, compiled: &[u8]) -> anyhow::Result<Module> {
#[allow(dead_code)]
unsafe fn load_compiled(&self, k: &Cid, compiled: &[u8]) -> anyhow::Result<Module> {
let k = self.with_redirect(k);
let mut cache = self
.inner
Expand All @@ -471,29 +443,6 @@ impl Engine {
Ok(module)
}

/// Lookup a loaded wasmtime module.
pub fn get_module(
&self,
blockstore: &impl Blockstore,
k: &Cid,
) -> anyhow::Result<Option<Module>> {
let k = self.with_redirect(k);
match self
.inner
.module_cache
.lock()
.expect("module_cache poisoned")
.entry(*k)
{
Occupied(v) => Ok(Some(v.get().module.clone())),
Vacant(v) => blockstore
.get(k)
.context("failed to lookup wasm module in blockstore")?
.map(|raw_wasm| Ok(v.insert(self.load_raw(&raw_wasm)?).module.clone()))
.transpose(),
}
}

/// Lookup and instantiate a loaded wasmtime module with the given store. This will cache the
/// linker, syscalls, etc.
///
Expand Down
2 changes: 1 addition & 1 deletion fvm/src/executor/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ where
// This interface works for now because we know all actor CIDs
// ahead of time, but with user-supplied code, we won't have that
// guarantee.
engine_pool.acquire().preload(
engine_pool.acquire().preload_all(
machine.blockstore(),
machine.builtin_actors().builtin_actor_codes(),
)?;
Expand Down
2 changes: 1 addition & 1 deletion fvm/src/kernel/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,7 @@ where
let size = self
.call_manager
.engine()
.preload(self.call_manager.blockstore(), &[code_id])
.preload_all(self.call_manager.blockstore(), &[code_id])
.context("failed to install actor")
.or_illegal_argument()?;

Expand Down
2 changes: 1 addition & 1 deletion testing/conformance/benches/bench_drivers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub fn bench_vector_variant(
// to do this explicitly.
engine
.acquire()
.preload(
.preload_all(
machine.blockstore(),
machine.builtin_actors().builtin_actor_codes(),
)
Expand Down
2 changes: 1 addition & 1 deletion testing/conformance/src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ pub fn run_variant(
// this explicitly.
engine
.acquire()
.preload(
.preload_all(
machine.blockstore(),
machine.builtin_actors().builtin_actor_codes(),
)
Expand Down
2 changes: 1 addition & 1 deletion testing/integration/src/tester.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ where
configure_mc(&mut mc);

let engine = EnginePool::new_default((&mc.network.clone()).into())?;
engine.acquire().preload(&blockstore, &self.code_cids)?;
engine.acquire().preload_all(&blockstore, &self.code_cids)?;

let machine = DefaultMachine::new(&mc, blockstore, externs)?;

Expand Down

0 comments on commit beb08d9

Please sign in to comment.