From 49b4504ed7eb9b878bc8291391114f201a36b9e3 Mon Sep 17 00:00:00 2001 From: vms Date: Wed, 10 Jun 2020 14:56:15 +0300 Subject: [PATCH] refactor FCE --- examples/ipfs_node/src/config.rs | 61 +++++--------- examples/ipfs_node/src/imports.rs | 29 +------ examples/ipfs_node/src/lib.rs | 1 - examples/ipfs_node/src/main.rs | 4 +- examples/ipfs_node/src/node.rs | 17 ++-- fce/src/lib.rs | 9 +++ fce/src/vm/fce.rs | 15 +--- fce/src/vm/module/fce_module.rs | 127 +++++++++--------------------- fce/src/vm/module/wit_function.rs | 21 +++-- fce/src/vm/module/wit_instance.rs | 9 +-- fce/src/wasm_process.rs | 11 +-- 11 files changed, 100 insertions(+), 204 deletions(-) diff --git a/examples/ipfs_node/src/config.rs b/examples/ipfs_node/src/config.rs index 5456eec3..02b56605 100644 --- a/examples/ipfs_node/src/config.rs +++ b/examples/ipfs_node/src/config.rs @@ -108,23 +108,7 @@ pub(crate) fn parse_config_from_file( .collect::>() }); - let wasi = module.wasi.map(|wasi| { - let envs = wasi - .envs - .map(|env| env.into_iter().map(|e| e.into_bytes()).collect::>()); - - let mapped_dirs = wasi.mapped_dirs.map(|mapped_dir| { - mapped_dir - .into_iter() - .map(|(from, to)| (from, to.try_into::().unwrap())) - .collect::>() - }); - WASIConfig { - envs, - preopened_files: wasi.preopened_files, - mapped_dirs, - } - }); + let wasi = module.wasi.map(parse_raw_wasi); ( module.name, ModuleConfig { @@ -138,23 +122,8 @@ pub(crate) fn parse_config_from_file( .collect::>(); let rpc_module_config = config.rpc_module.map(|rpc_module| { - let wasi = rpc_module.wasi.map(|wasi| { - let envs = wasi - .envs - .map(|env| env.into_iter().map(|e| e.into_bytes()).collect::>()); + let wasi = rpc_module.wasi.map(parse_raw_wasi); - let mapped_dirs = wasi.mapped_dirs.map(|mapped_dir| { - mapped_dir - .into_iter() - .map(|(from, to)| (from, to.try_into::().unwrap())) - .collect::>() - }); - WASIConfig { - envs, - preopened_files: wasi.preopened_files, - mapped_dirs, - } - }); ModuleConfig { mem_pages_count: rpc_module.mem_pages_count, logger_enabled: rpc_module.logger_enabled, @@ -163,15 +132,27 @@ pub(crate) fn parse_config_from_file( } }); - /* - println!( - "parsed modules config:\n{:?}\nparsed rpc config:\n{:?}", - modules_config, rpc_module_config - ); - */ - Ok(NodeConfig { modules_config, rpc_module_config, }) } + +fn parse_raw_wasi(wasi: RawWASIConfig) -> WASIConfig { + let envs = wasi + .envs + .map(|env| env.into_iter().map(|e| e.into_bytes()).collect::>()); + + let mapped_dirs = wasi.mapped_dirs.map(|mapped_dir| { + mapped_dir + .into_iter() + .map(|(from, to)| (from, to.try_into::().unwrap())) + .collect::>() + }); + + WASIConfig { + envs, + preopened_files: wasi.preopened_files, + mapped_dirs, + } +} diff --git a/examples/ipfs_node/src/imports.rs b/examples/ipfs_node/src/imports.rs index b716b2e2..6e709eec 100644 --- a/examples/ipfs_node/src/imports.rs +++ b/examples/ipfs_node/src/imports.rs @@ -23,43 +23,20 @@ pub(super) fn log_utf8_string(ctx: &mut Ctx, offset: i32, size: i32) { let wasm_ptr = WasmPtr::::new(offset as _); match wasm_ptr.get_utf8_string(ctx.memory(0), size as _) { Some(msg) => print!("{}", msg), - None => print!("ipfs node logger: incorrect UTF8 string's been supplied to logger\n"), + None => println!("ipfs node logger: incorrect UTF8 string's been supplied to logger"), } } -pub(super) fn ipfs(ctx: &mut Ctx, offset: i32, size: i32) -> i32 { - use wasmer_core::memory::ptr::{Array, WasmPtr}; - - let wasm_ptr = WasmPtr::::new(offset as _); - match wasm_ptr.get_utf8_string(ctx.memory(0), size as _) { - Some(msg) => println!("ipfs"), - None => println!("ipfs\n"), - } - - 0x1337 -} - -#[derive(Clone)] -struct T {} - -impl Drop for T { - fn drop(&mut self) { - println!("drop T"); - } -} - -pub(super) fn create_host_import_func(host_cmd: String) -> DynamicFunc<'static> { +pub(super) fn create_host_import_func(_host_cmd: String) -> DynamicFunc<'static> { use wasmer_core::types::Value; use wasmer_core::types::Type; use wasmer_core::types::FuncSig; - let t = T{}; let func = move |ctx: &mut Ctx, inputs: &[Value]| -> Vec { use wasmer_core::memory::ptr::{Array, WasmPtr}; - let _t = t.clone(); - println!("inputs size is {}", inputs.len()); + // TODO: refactor this let array_ptr = inputs[1].to_u128() as i32; let array_size = inputs[0].to_u128() as i32; diff --git a/examples/ipfs_node/src/lib.rs b/examples/ipfs_node/src/lib.rs index 8ccaaa26..f83a8c0c 100644 --- a/examples/ipfs_node/src/lib.rs +++ b/examples/ipfs_node/src/lib.rs @@ -21,4 +21,3 @@ mod imports; pub use node::IpfsNode; pub use node::NodeModule; -pub(crate) use imports::log_utf8_string; diff --git a/examples/ipfs_node/src/main.rs b/examples/ipfs_node/src/main.rs index bcf2205e..4c2f856b 100644 --- a/examples/ipfs_node/src/main.rs +++ b/examples/ipfs_node/src/main.rs @@ -39,7 +39,9 @@ fn main() { ) .unwrap(); - let result = ipfs_node.rpc_call(&ipfs_rpc).unwrap(); + println!("ipfs node interface is {:?}", ipfs_node.get_interface()); + + let result = ipfs_node.rpc_call(&ipfs_rpc, &[]).unwrap(); println!("execution result {:?}", result); } diff --git a/examples/ipfs_node/src/node.rs b/examples/ipfs_node/src/node.rs index 4c96b176..ad0b117d 100644 --- a/examples/ipfs_node/src/node.rs +++ b/examples/ipfs_node/src/node.rs @@ -42,12 +42,12 @@ pub struct NodeModule<'a> { } impl IpfsNode { - pub fn new(core_modules_dir: PathBuf, config_file_path: PathBuf) -> Result { + pub fn new>(core_modules_dir: P, config_file_path: P) -> Result { let mut wasm_process = FCE::new(); let mut module_names = Vec::new(); - let mut core_modules_config = crate::config::parse_config_from_file(config_file_path)?; + let mut core_modules_config = crate::config::parse_config_from_file(config_file_path.into())?; - for entry in fs::read_dir(core_modules_dir)? { + for entry in fs::read_dir(core_modules_dir.into())? { let path = entry?.path(); if path.is_dir() { continue; @@ -62,12 +62,12 @@ impl IpfsNode { let module_bytes = fs::read(path.clone())?; let core_module_config = - Self::make_wasm_config(core_modules_config.modules_config.remove(&module_name))?; + Self::make_wasm_process_config(core_modules_config.modules_config.remove(&module_name))?; wasm_process.load_module(module_name.clone(), &module_bytes, core_module_config)?; module_names.push(module_name); } - let rpc_module_config = Self::make_wasm_config(core_modules_config.rpc_module_config)?; + let rpc_module_config = Self::make_wasm_process_config(core_modules_config.rpc_module_config)?; Ok(Self { process: wasm_process, @@ -76,7 +76,7 @@ impl IpfsNode { }) } - pub fn rpc_call(&mut self, wasm_rpc: &[u8]) -> Result, NodeError> { + pub fn rpc_call(&mut self, wasm_rpc: &[u8], args: &[IValue]) -> Result, NodeError> { let rpc_module_name = "ipfs_rpc"; self.process @@ -85,7 +85,7 @@ impl IpfsNode { let call_result = self.process.call( rpc_module_name, "invoke", - &[IValue::String("test".to_string())], + args )?; self.process.unload_module(rpc_module_name)?; @@ -106,7 +106,7 @@ impl IpfsNode { modules } - fn make_wasm_config(config: Option) -> Result { + fn make_wasm_process_config(config: Option) -> Result { use crate::imports::create_host_import_func; use crate::imports::log_utf8_string; use wasmer_core::import::Namespace; @@ -134,7 +134,6 @@ impl IpfsNode { for (import_name, host_cmd) in imports { let host_import = create_host_import_func(host_cmd); namespace.insert(import_name, host_import); - //namespace.insert(import_name, func!(crate::imports::ipfs)); } } diff --git a/fce/src/lib.rs b/fce/src/lib.rs index 32ce8d8a..e34be3b9 100644 --- a/fce/src/lib.rs +++ b/fce/src/lib.rs @@ -16,6 +16,15 @@ #![warn(rust_2018_idioms)] #![feature(get_mut_unchecked)] #![feature(new_uninit)] +#![deny( + dead_code, + nonstandard_style, + unused_imports, + unused_mut, + unused_variables, + unused_unsafe, + unreachable_patterns +)] mod vm; mod misc; diff --git a/fce/src/vm/fce.rs b/fce/src/vm/fce.rs index 83b413af..d7c0d31d 100644 --- a/fce/src/vm/fce.rs +++ b/fce/src/vm/fce.rs @@ -19,21 +19,15 @@ use super::*; use crate::WasmProcess; use crate::NodeFunction; -use std::sync::Arc; use std::collections::hash_map::Entry; use std::collections::HashMap; +/// The base struct for the Fluence Compute Engine. pub struct FCE { // set of modules registered inside FCE modules: HashMap, } -impl Drop for FCE { - fn drop(&mut self) { - println!("FCE dropped"); - } -} - impl FCE { pub fn new() -> Self { Self { @@ -57,11 +51,8 @@ impl WasmProcess for FCE { ) -> Result, FCEError> { match self.modules.get_mut(module_name) { // TODO: refactor errors - Some(mut module) => module.call(func_name, argument), - None => { - println!("no such module"); - Err(FCEError::NoSuchModule) - } + Some(module) => module.call(func_name, argument), + None => Err(FCEError::NoSuchModule), } } diff --git a/fce/src/vm/module/fce_module.rs b/fce/src/vm/module/fce_module.rs index 09874870..0ccf4c86 100644 --- a/fce/src/vm/module/fce_module.rs +++ b/fce/src/vm/module/fce_module.rs @@ -28,7 +28,6 @@ use std::collections::HashMap; use std::convert::TryInto; use std::mem::MaybeUninit; use std::sync::Arc; -use std::borrow::BorrowMut; type WITInterpreter = Interpreter>; @@ -50,7 +49,8 @@ impl Callable { pub fn call(&mut self, args: &[IValue]) -> Result, FCEError> { use wasmer_wit::interpreter::stack::Stackable; - let result = self.wit_module_func + let result = self + .wit_module_func .interpreter .run(args, Arc::make_mut(&mut self.wit_instance))? .as_slice() @@ -61,26 +61,24 @@ impl Callable { } pub struct FCEModule { - // it is needed because of WITInstance contains dynamic functions + // wasmer_instance is needed because WITInstance contains dynamic functions // that internally keep pointer to Wasmer instance. #[allow(unused)] - wamser_instance: WasmerInstance, + wamser_instance: Box, + + // import_object is needed because ImportObject::extend doesn't really deep copy + // imports, so we need to store imports of this module to prevent their removing. + #[allow(unused)] import_object: ImportObject, // TODO: replace with dyn Trait - pub(super) exports_funcs: HashMap>, -} - -impl Drop for FCEModule { - fn drop(&mut self) { - println!("FCEModule dropped: {:?}", self.exports_funcs.keys()); - } + exports_funcs: HashMap>, } impl FCEModule { pub fn new( wasm_bytes: &[u8], - fce_imports: ImportObject, + mut fce_imports: ImportObject, modules: &HashMap, ) -> Result { let wasmer_module = compile(&wasm_bytes)?; @@ -88,14 +86,10 @@ impl FCEModule { let fce_wit = FCEWITInterfaces::new(wit); let mut wit_instance = Arc::new_uninit(); - let mut import_object = Self::adjust_wit_imports(&fce_wit, wit_instance.clone())?; - let mut fce_imports = fce_imports; - + let import_object = Self::adjust_wit_imports(&fce_wit, wit_instance.clone())?; fce_imports.extend(import_object.clone()); - let wasmer_instance = wasmer_module.instantiate(&fce_imports)?; - let wit_instance = unsafe { // get_mut_unchecked here is safe because currently only this modules have reference to // it and the environment is single-threaded @@ -104,22 +98,18 @@ impl FCEModule { std::mem::transmute::<_, Arc>(wit_instance) }; - let exports_funcs = Self::instantiate_wit_exports(wit_instance.clone(), &fce_wit)?; + let exports_funcs = Self::instantiate_wit_exports(wit_instance, &fce_wit)?; Ok(Self { - wamser_instance: wasmer_instance, + wamser_instance: Box::new(wasmer_instance), import_object, exports_funcs, }) } pub fn call(&mut self, function_name: &str, args: &[IValue]) -> Result, FCEError> { - use wasmer_wit::interpreter::stack::Stackable; - match self.exports_funcs.get_mut(function_name) { - Some(func) => { - Arc::make_mut(func).call(args) - } + Some(func) => Arc::make_mut(func).call(args), None => Err(FCEError::NoSuchFunction(format!( "{} hasn't been found while calling", function_name @@ -127,25 +117,6 @@ impl FCEModule { } } - pub fn get_func_signature( - &self, - function_name: &str, - ) -> Result<(&Vec, &Vec), FCEError> { - match self.exports_funcs.get(function_name) { - Some(func) => Ok((&func.wit_module_func.inputs, &func.wit_module_func.outputs)), - None => { - for func in self.exports_funcs.iter() { - println!("{}", func.0); - } - - Err(FCEError::NoSuchFunction(format!( - "{} has't been found during its signature looking up", - function_name - ))) - } - } - } - pub fn get_exports_signatures( &self, ) -> impl Iterator, &Vec)> { @@ -158,6 +129,17 @@ impl FCEModule { }) } + // TODO: change the cloning Callable behaviour after changes of Wasmer API + pub(super) fn get_callable(&self, function_name: &str) -> Result, FCEError> { + match self.exports_funcs.get(function_name) { + Some(func) => Ok(func.clone()), + None => Err(FCEError::NoSuchFunction(format!( + "{} hasn't been found while calling", + function_name + ))), + } + } + fn instantiate_wit_exports( wit_instance: Arc, wit: &FCEWITInterfaces<'_>, @@ -221,58 +203,24 @@ impl FCEModule { use wasmer_core::typed_func::DynamicFunc; use wasmer_core::vm::Ctx; - #[derive(Clone)] - struct T {} - - impl Drop for T { - fn drop(&mut self) { - println!("fce_module imports: drop T"); - } - } - // returns function that will be called from imports of Wasmer module - fn dyn_func_from_raw_import( - inputs: Vec, - wit_instance: Arc>, - interpreter: WITInterpreter, - ) -> DynamicFunc<'static> { + fn dyn_func_from_raw_import(inputs: Vec, raw_import: F) -> DynamicFunc<'static> + where + F: Fn(&mut Ctx, &[WValue]) -> Vec + 'static, + { use wasmer_core::types::FuncSig; use super::type_converters::itype_to_wtype; - let t = T {}; let signature = inputs.iter().map(itype_to_wtype).collect::>(); - DynamicFunc::new( - Arc::new(FuncSig::new(signature, vec![])), - move |_: &mut Ctx, inputs: &[WValue]| -> Vec { - use super::type_converters::wval_to_ival; - let t_copied = t.clone(); - - println!("dyn_func_from_raw_import: {:?}", inputs); - - // copy here because otherwise wit_instance will be consumed by the closure - let wit_instance_callable = wit_instance.clone(); - let converted_inputs = inputs.iter().map(wval_to_ival).collect::>(); - unsafe { - // error here will be propagated by the special error instruction - let _ = interpreter.run( - &converted_inputs, - Arc::make_mut(&mut wit_instance_callable.assume_init()), - ); - } - - // wit import functions should only change the stack state - - // the result will be returned by an export function - vec![] - }, - ) + DynamicFunc::new(Arc::new(FuncSig::new(signature, vec![])), raw_import) } // creates a closure that is represent a WIT module import fn create_raw_import( wit_instance: Arc>, interpreter: WITInterpreter, - ) -> Box Fn(&'a mut Ctx, &'b [WValue]) -> Vec + 'static> { - Box::new(move |_: &mut Ctx, inputs: &[WValue]| -> Vec { + ) -> impl Fn(&mut Ctx, &[WValue]) -> Vec + 'static { + move |_: &mut Ctx, inputs: &[WValue]| -> Vec { use super::type_converters::wval_to_ival; // copy here because otherwise wit_instance will be consumed by the closure @@ -289,7 +237,7 @@ impl FCEModule { // wit import functions should only change the stack state - // the result will be returned by an export function vec![] - }) + } } let wit_import_funcs = wit @@ -315,11 +263,8 @@ impl FCEModule { WITAstType::Function { inputs, .. } => { let interpreter: WITInterpreter = adapter_instructions.try_into()?; - let wit_import = dyn_func_from_raw_import( - inputs.clone(), - wit_instance.clone(), - interpreter, - ); + let raw_import = create_raw_import(wit_instance.clone(), interpreter); + let wit_import = dyn_func_from_raw_import(inputs.clone(), raw_import); Ok((import_namespace.to_string(), (*import_name, wit_import))) } @@ -333,7 +278,7 @@ impl FCEModule { let mut import_object = ImportObject::new(); - // TODO: refactor it + // TODO: refactor this for (namespace_name, funcs) in wit_import_funcs.into_iter() { let mut namespace = Namespace::new(); for (import_name, import_func) in funcs.into_iter() { diff --git a/fce/src/vm/module/wit_function.rs b/fce/src/vm/module/wit_function.rs index 1ede5093..d803ef05 100644 --- a/fce/src/vm/module/wit_function.rs +++ b/fce/src/vm/module/wit_function.rs @@ -50,7 +50,10 @@ impl Drop for WITFunction { println!("WITFunction export dropped: {:?}", func.signature()); } WITFunctionInner::Import { callable } => { - println!("WITFunction import dropped: {:?}", callable.wit_module_func.inputs); + println!( + "WITFunction import dropped: {:?}", + callable.wit_module_func.inputs + ); } } } @@ -85,13 +88,11 @@ impl WITFunction { /// Creates function from a module import. pub(super) fn from_import( wit_module: &FCEModule, - func_name: String, + function_name: &str, ) -> Result { - let callable = wit_module.exports_funcs.get(&func_name).unwrap().clone(); + let callable = wit_module.get_callable(function_name)?; - let inner = WITFunctionInner::Import { - callable - }; + let inner = WITFunctionInner::Import { callable }; Ok(Self { inner }) } @@ -135,11 +136,9 @@ impl wasm::structures::LocalImport for WITFunction { .call(&arguments.iter().map(ival_to_wval).collect::>()) .map(|result| result.iter().map(wval_to_ival).collect()) .map_err(|_| ()), - WITFunctionInner::Import { - callable - } => { - Arc::make_mut(&mut callable.clone()).call(arguments).map_err(|_| ()) - } + WITFunctionInner::Import { callable } => Arc::make_mut(&mut callable.clone()) + .call(arguments) + .map_err(|_| ()), } } } diff --git a/fce/src/vm/module/wit_instance.rs b/fce/src/vm/module/wit_instance.rs index bc2487e8..5f98c140 100644 --- a/fce/src/vm/module/wit_instance.rs +++ b/fce/src/vm/module/wit_instance.rs @@ -24,7 +24,6 @@ use wasmer_wit::interpreter::wasm::structures::{LocalImportIndex, TypedIndex}; use wasmer_core::Instance as WasmerInstance; use std::collections::HashMap; -use std::sync::Arc; /// Contains all import and export functions that could be called from WIT context by call-core. #[derive(Clone)] @@ -33,12 +32,6 @@ pub(super) struct WITInstance { memories: Vec, } -impl Drop for WITInstance { - fn drop(&mut self) { - println!("WITInstance dropped"); - } -} - impl WITInstance { pub(super) fn new( wasmer_instance: &WasmerInstance, @@ -92,7 +85,7 @@ impl WITInstance { .enumerate() .map(|(idx, import)| match modules.get(import.namespace) { Some(module) => { - let func = WITFunction::from_import(module, import.name.to_string())?; + let func = WITFunction::from_import(module, import.name)?; Ok((start_index + idx as usize, func)) } None => Err(FCEError::NoSuchModule), diff --git a/fce/src/wasm_process.rs b/fce/src/wasm_process.rs index 249c087f..0f66d4e3 100644 --- a/fce/src/wasm_process.rs +++ b/fce/src/wasm_process.rs @@ -19,6 +19,7 @@ use super::FCEError; use super::IValue; use super::IType; +/// Represent a function type inside wasm process. #[derive(Debug)] pub struct NodeFunction<'a> { pub name: &'a str, @@ -26,9 +27,9 @@ pub struct NodeFunction<'a> { pub outputs: &'a Vec, } -/// Describes a run computation node behaviour in the Fluence network. +/// Describe a computation node behaviour in the Fluence network. pub trait WasmProcess { - /// Invokes a module supplying byte array and expecting byte array with some outcome back. + /// Invoke a function by given function name with given arguments of a module inside wasm process. fn call( &mut self, module_name: &str, @@ -36,7 +37,7 @@ pub trait WasmProcess { arguments: &[IValue], ) -> Result, FCEError>; - /// Loads new module in the FCE Service. + /// Load a new module inside wasm process. fn load_module( &mut self, module_name: S, @@ -46,9 +47,9 @@ pub trait WasmProcess { where S: Into; - /// Unloads previously registered module. + /// Unload previously loaded module. fn unload_module(&mut self, module_name: &str) -> Result<(), FCEError>; - /// Returns signatures of all exported functions by this module. + /// Return signatures of all exported by this module functions. fn get_interface(&self, module_name: &str) -> Result>, FCEError>; }