second-state / wasmedge-bindgen

Let WebAssembly's exported function support more data types for its parameters and return values.
Apache License 2.0
29 stars 8 forks source link

Support wasmedge sdk #5

Closed chenyukang closed 1 year ago

chenyukang commented 2 years ago

This is a WIP PR, if we want to support WasmEdge Rust SDK, only some trivial API names need to fix.

The main diff is:

diff --git a/host/rust-wasmedge-sdk/src/lib.rs b/host/rust-wasmedge-sdk/src/lib.rs
index 1ea7c5e..00a5183 100644
--- a/host/rust-wasmedge-sdk/src/lib.rs
+++ b/host/rust-wasmedge-sdk/src/lib.rs
@@ -2,10 +2,10 @@ use std::any::Any;
 use std::ptr::NonNull;
 use core::ops::{Deref, DerefMut};

-use num_derive::FromPrimitive;    
+use num_derive::FromPrimitive;
 use num_traits::FromPrimitive;
-use wasmedge_sys::*;
 use wasmedge_types::*;
+use wasmedge_sdk::{*, types::WasmValue};

 pub enum Param<'a> {
    I8(i8),
@@ -36,69 +36,69 @@ impl<'a> Param<'a> {
            Param::I8(v) => {
                let length = 1;
                let pointer = allocate(vm, length)?;
-               mem.set_data(vec![*v as u8], pointer as u32)?;
+               mem.write(vec![*v as u8], pointer as u32)?;
                Ok((pointer, length))
            }
            Param::U8(v) => {
                let length = 1;
                let pointer = allocate(vm, length)?;
-               mem.set_data(vec![*v], pointer as u32)?;
+               mem.write(vec![*v], pointer as u32)?;
                Ok((pointer, length))
            }
            Param::I16(v) => {
                let length = 1;
                let pointer = allocate(vm, length * 2)?;
                let bytes = v.to_le_bytes();
-               mem.set_data(bytes, pointer as u32)?;
+               mem.write(bytes, pointer as u32)?;
                Ok((pointer, length))
            }
            Param::U16(v) => {
                let length = 2;
                let pointer = allocate(vm, length * 2)?;
                let bytes = v.to_le_bytes();
-               mem.set_data(bytes, pointer as u32)?;
+               mem.write(bytes, pointer as u32)?;
                Ok((pointer, length))
            }
            Param::I32(v) => {
                let length = 1;
                let pointer = allocate(vm, length * 4)?;
                let bytes = v.to_le_bytes();
-               mem.set_data(bytes, pointer as u32)?;
+               mem.write(bytes, pointer as u32)?;
                Ok((pointer, length))
            }
            Param::U32(v) => {
                let length = 1;
                let pointer = allocate(vm, length * 4)?;
                let bytes = v.to_le_bytes();
-               mem.set_data(bytes, pointer as u32)?;
+               mem.write(bytes, pointer as u32)?;
                Ok((pointer, length))
            }
            Param::I64(v) => {
                let length = 1;
                let pointer = allocate(vm, length * 8)?;
                let bytes = v.to_le_bytes();
-               mem.set_data(bytes, pointer as u32)?;
+               mem.write(bytes, pointer as u32)?;
                Ok((pointer, length))
            }
            Param::U64(v) => {
                let length = 1;
                let pointer = allocate(vm, length * 8)?;
                let bytes = v.to_le_bytes();
-               mem.set_data(bytes, pointer as u32)?;
+               mem.write(bytes, pointer as u32)?;
                Ok((pointer, length))
            }
            Param::F32(v) => {
                let length = 1;
                let pointer = allocate(vm, length * 4)?;
                let bytes = v.to_le_bytes();
-               mem.set_data(bytes, pointer as u32)?;
+               mem.write(bytes, pointer as u32)?;
                Ok((pointer, length))
            }
            Param::F64(v) => {
                let length = 1;
                let pointer = allocate(vm, length * 8)?;
                let bytes = v.to_le_bytes();
-               mem.set_data(bytes, pointer as u32)?;
+               mem.write(bytes, pointer as u32)?;
                Ok((pointer, length))
            }
            Param::Bool(v) => {
@@ -108,7 +108,7 @@ impl<'a> Param<'a> {
                    true => 1,
                    false => 0
                };
-               mem.set_data(vec![byte], pointer as u32)?;
+               mem.write(vec![byte], pointer as u32)?;
                Ok((pointer, length))
            }
            Param::VecI8(v) => {
@@ -118,7 +118,7 @@ impl<'a> Param<'a> {
                for (pos, iv) in v.iter().enumerate() {
                    bytes[pos] = *iv as u8;
                }
-               mem.set_data(bytes, pointer as u32)?;
+               mem.write(bytes, pointer as u32)?;
                Ok((pointer, length))
            }
            Param::VecU8(v) => {
@@ -128,7 +128,7 @@ impl<'a> Param<'a> {
                for (pos, iv) in v.iter().enumerate() {
                    bytes[pos] = *iv;
                }
-               mem.set_data(bytes, pointer as u32)?;
+               mem.write(bytes, pointer as u32)?;
                Ok((pointer, length))
            }
            Param::VecI16(v) => {
@@ -141,7 +141,7 @@ impl<'a> Param<'a> {
                        bytes[pos * 2 + i] = b[i];
                    }
                }
-               mem.set_data(bytes, pointer as u32)?;
+               mem.write(bytes, pointer as u32)?;
                Ok((pointer, length))
            }
            Param::VecU16(v) => {
@@ -154,7 +154,7 @@ impl<'a> Param<'a> {
                        bytes[pos * 2 + i] = b[i];
                    }
                }
-               mem.set_data(bytes, pointer as u32)?;
+               mem.write(bytes, pointer as u32)?;
                Ok((pointer, length))
            }
            Param::VecI32(v) => {
@@ -167,7 +167,7 @@ impl<'a> Param<'a> {
                        bytes[pos * 4 + i] = b[i];
                    }
                }
-               mem.set_data(bytes, pointer as u32)?;
+               mem.write(bytes, pointer as u32)?;
                Ok((pointer, length))
            }
            Param::VecU32(v) => {
@@ -180,7 +180,7 @@ impl<'a> Param<'a> {
                        bytes[pos * 4 + i] = b[i];
                    }
                }
-               mem.set_data(bytes, pointer as u32)?;
+               mem.write(bytes, pointer as u32)?;
                Ok((pointer, length))
            }
            Param::VecI64(v) => {
@@ -193,7 +193,7 @@ impl<'a> Param<'a> {
                        bytes[pos * 8 + i] = b[i];
                    }
                }
-               mem.set_data(bytes, pointer as u32)?;
+               mem.write(bytes, pointer as u32)?;
                Ok((pointer, length))
            }
            Param::VecU64(v) => {
@@ -206,14 +206,14 @@ impl<'a> Param<'a> {
                        bytes[pos * 8 + i] = b[i];
                    }
                }
-               mem.set_data(bytes, pointer as u32)?;
+               mem.write(bytes, pointer as u32)?;
                Ok((pointer, length))
            }
            Param::String(v) => {
                let bytes = v.as_bytes().to_vec();
                let length = bytes.len() as i32;
                let pointer = allocate(vm, length)?;
-               mem.set_data(bytes, pointer as u32)?;
+               mem.write(bytes, pointer as u32)?;
                Ok((pointer, length))
            }
        }
@@ -339,7 +339,7 @@ impl Bindgen {
            }
        };

-       let mut memory = self.vm.active_module().unwrap().get_memory("memory").unwrap();
+       let mut memory = self.vm.active_module().unwrap().memory("memory").unwrap();

        for (pos, inp) in inputs.iter().enumerate() {
            let sr = inp.settle(&self.vm, &mut memory);
@@ -353,8 +353,8 @@ impl Bindgen {
                }
            };

-           memory.set_data(pointer.to_le_bytes(), pointer_of_pointers as u32 + pos as u32 * 4 * 2)?;
-           memory.set_data(length_of_input.to_le_bytes(), pointer_of_pointers as u32 + pos as u32 * 4 * 2 + 4)?;
+           memory.write(pointer.to_le_bytes(), pointer_of_pointers as u32 + pos as u32 * 4 * 2)?;
+           memory.write(length_of_input.to_le_bytes(), pointer_of_pointers as u32 + pos as u32 * 4 * 2 + 4)?;
        }

        let rets = self.vm.run_function(func_name, vec![WasmValue::from_i32(pointer_of_pointers), WasmValue::from_i32(inputs_count)])?;
@@ -364,7 +364,7 @@ impl Bindgen {
        if rets.len() != 1 {
            return Ok(Err(String::from("Invalid return value")));
        }
-       let rvec = memory.get_data(rets[0].to_i32() as u32, 9)?;
+       let rvec = memory.read(rets[0].to_i32() as u32, 9)?;
        let _ = self.vm.run_function("deallocate", vec![WasmValue::from_i32(rets[0].to_i32()), WasmValue::from_i32(9)]);

        let flag = rvec[0];
@@ -377,16 +377,16 @@ impl Bindgen {
    }

    fn parse_error(&self, ret_pointer: i32, ret_len: i32) -> String {
-       let memory = self.vm.active_module().unwrap().get_memory("memory").unwrap();
-       let err_bytes = memory.get_data(ret_pointer as u32, ret_len as u32).unwrap();
+       let memory = self.vm.active_module().unwrap().memory("memory").unwrap();
+       let err_bytes = memory.read(ret_pointer as u32, ret_len as u32).unwrap();
        let _ = self.vm.run_function("deallocate", vec![WasmValue::from_i32(ret_pointer), WasmValue::from_i32(ret_len)]);
        String::from_utf8(err_bytes).unwrap_or_default()
    }

    fn parse_result(&self, ret_pointer: i32, ret_len: i32) -> Vec<Box<dyn Any + Send + Sync>> {
        let size = ret_len as usize;
-       let memory = self.vm.active_module().unwrap().get_memory("memory").unwrap();
-       let p_data = memory.get_data(ret_pointer as u32, size as u32 * 3 * 4).unwrap();
+       let memory = self.vm.active_module().unwrap().memory("memory").unwrap();
+       let p_data = memory.read(ret_pointer as u32, size as u32 * 3 * 4).unwrap();
        let _ = self.vm.run_function("deallocate", vec![WasmValue::from_i32(ret_pointer), WasmValue::from_i32(size as i32 * 3 * 4)]);

        let mut p_values = vec![0; size * 3];
@@ -398,7 +398,7 @@ impl Bindgen {
        let mut results: Vec<Box<dyn Any + Send + Sync>> = Vec::with_capacity(size);

        for i in 0..size {
-           let bytes = memory.get_data(p_values[i*3] as u32, p_values[i*3+2] as u32).unwrap();
+           let bytes = memory.read(p_values[i*3] as u32, p_values[i*3+2] as u32).unwrap();
            let _ = self.vm.run_function("deallocate", vec![WasmValue::from_i32(p_values[i*3]), WasmValue::from_i32(p_values[i*3+2])]);
            match FromPrimitive::from_i32(p_values[i*3+1]) {
                Some(RetTypes::U8) => {
chenyukang commented 2 years ago

I created two sub-projects to demo how we can make it work with wasmedge-sys and wasmedge-sdk.

If we keep the API from SDK VM and Memory the same with inner data structure, such as get_data, read_data, get_memory, we could have one project to support both wasmedge-sys and wasmedge-sdk?

Any ideas on how we plan to integrate wasmedge-sdk with wasmedge-bindgen? @DarumaDocker

chenyukang commented 2 years ago

To make changes as small as possible, I also make some temporary changes on wasmedge-sdk:

diff --git a/src/types.rs b/src/types.rs
index b7fbc17..54e2113 100644
--- a/src/types.rs
+++ b/src/types.rs
@@ -1,7 +1,7 @@
 //! Defines the general types.

 use crate::FuncRef;
-use wasmedge_sys::WasmValue;
+pub use wasmedge_sys::WasmValue;
 use wasmedge_types::{self, RefType};

 /// Defines runtime values that a WebAssembly module can either consume or produce.
diff --git a/src/vm.rs b/src/vm.rs
index 56d17af..4136320 100644
--- a/src/vm.rs
+++ b/src/vm.rs
@@ -131,6 +131,18 @@ impl Vm {
         Ok(self)
     }

+    pub fn load_wasm_from_file(&mut self, path: impl AsRef<Path>) -> WasmEdgeResult<()> {
+        self.inner.load_wasm_from_file(path.as_ref())
+    }
+
+    pub fn wasi_module_mut(&mut self) -> WasmEdgeResult<wasmedge_sys::instance::module::WasiModule> {
+        self.inner.wasi_module_mut()
+    }
+
+    pub fn validate(&mut self) -> WasmEdgeResult<()> {
+        self.inner.validate()
+    }
+
     /// Registers a WASM module from then given in-memory wasm bytes into the [Vm], and instantiates it.
     ///
     /// # Arguments
@@ -207,6 +219,10 @@ impl Vm {
         Ok(self)
     }

+    pub fn instantiate(&mut self) -> WasmEdgeResult<()> {
+        self.inner.instantiate()
+    }
+
     /// Resets the [Vm].
     pub fn reset(&mut self) {
         self.inner.reset()
@@ -246,6 +262,15 @@ impl Vm {
         Ok(returns)
     }

+    pub fn run_function(
+        &self,
+        func_name: impl AsRef<str>,
+        params: impl IntoIterator<Item = sys::WasmValue>,
+    ) -> WasmEdgeResult<Vec<sys::WasmValue>> {
+        self.inner.run_function(func_name.as_ref(), params)
+    }
+
+
     /// Returns the type of a WASM function.
     ///
     /// # Arguments
juntao commented 2 years ago

@apepkuss Can you review this? The goal is to support wasmedge-bindgen in our Rust SDK.

apepkuss commented 2 years ago

To make changes as small as possible, I also make some temporary changes on wasmedge-sdk:

diff --git a/src/types.rs b/src/types.rs
index b7fbc17..54e2113 100644
--- a/src/types.rs
+++ b/src/types.rs
@@ -1,7 +1,7 @@
 //! Defines the general types.

 use crate::FuncRef;
-use wasmedge_sys::WasmValue;
+pub use wasmedge_sys::WasmValue;
 use wasmedge_types::{self, RefType};

 /// Defines runtime values that a WebAssembly module can either consume or produce.
diff --git a/src/vm.rs b/src/vm.rs
index 56d17af..4136320 100644
--- a/src/vm.rs
+++ b/src/vm.rs
@@ -131,6 +131,18 @@ impl Vm {
         Ok(self)
     }

+    pub fn load_wasm_from_file(&mut self, path: impl AsRef<Path>) -> WasmEdgeResult<()> {
+        self.inner.load_wasm_from_file(path.as_ref())
+    }
+
+    pub fn wasi_module_mut(&mut self) -> WasmEdgeResult<wasmedge_sys::instance::module::WasiModule> {
+        self.inner.wasi_module_mut()
+    }
+
+    pub fn validate(&mut self) -> WasmEdgeResult<()> {
+        self.inner.validate()
+    }
+
     /// Registers a WASM module from then given in-memory wasm bytes into the [Vm], and instantiates it.
     ///
     /// # Arguments
@@ -207,6 +219,10 @@ impl Vm {
         Ok(self)
     }

+    pub fn instantiate(&mut self) -> WasmEdgeResult<()> {
+        self.inner.instantiate()
+    }
+
     /// Resets the [Vm].
     pub fn reset(&mut self) {
         self.inner.reset()
@@ -246,6 +262,15 @@ impl Vm {
         Ok(returns)
     }

+    pub fn run_function(
+        &self,
+        func_name: impl AsRef<str>,
+        params: impl IntoIterator<Item = sys::WasmValue>,
+    ) -> WasmEdgeResult<Vec<sys::WasmValue>> {
+        self.inner.run_function(func_name.as_ref(), params)
+    }
+
+
     /// Returns the type of a WASM function.
     ///
     /// # Arguments

As the API design principles of wasmedge-sdk and wasmedge-sys are different, the wasmedge-bindgen should provide supports for them, respectively. It is not recommended that refactoring the existing APIs of wasmedge-sdk and wasmedge-sys to simplify the design of wasmedge-bindgen. I strongly recommend that wasmedge-bindgen should focus on the support for wasmedge-sdk, because wasmedge-sys is a low-level layer in the architecture of WasmEdge Rust bindings, we should help the early-phase end users migrate their projects onto wasmedge-sdk. Therefore, it would be much better to provide full support for wasmedge-sdk and weaken the support for wasmedge-sys gradually.

chenyukang commented 2 years ago

To make changes as small as possible, I also make some temporary changes on wasmedge-sdk:

diff --git a/src/types.rs b/src/types.rs
index b7fbc17..54e2113 100644
--- a/src/types.rs
+++ b/src/types.rs
@@ -1,7 +1,7 @@
 //! Defines the general types.

 use crate::FuncRef;
-use wasmedge_sys::WasmValue;
+pub use wasmedge_sys::WasmValue;
 use wasmedge_types::{self, RefType};

 /// Defines runtime values that a WebAssembly module can either consume or produce.
diff --git a/src/vm.rs b/src/vm.rs
index 56d17af..4136320 100644
--- a/src/vm.rs
+++ b/src/vm.rs
@@ -131,6 +131,18 @@ impl Vm {
         Ok(self)
     }

+    pub fn load_wasm_from_file(&mut self, path: impl AsRef<Path>) -> WasmEdgeResult<()> {
+        self.inner.load_wasm_from_file(path.as_ref())
+    }
+
+    pub fn wasi_module_mut(&mut self) -> WasmEdgeResult<wasmedge_sys::instance::module::WasiModule> {
+        self.inner.wasi_module_mut()
+    }
+
+    pub fn validate(&mut self) -> WasmEdgeResult<()> {
+        self.inner.validate()
+    }
+
     /// Registers a WASM module from then given in-memory wasm bytes into the [Vm], and instantiates it.
     ///
     /// # Arguments
@@ -207,6 +219,10 @@ impl Vm {
         Ok(self)
     }

+    pub fn instantiate(&mut self) -> WasmEdgeResult<()> {
+        self.inner.instantiate()
+    }
+
     /// Resets the [Vm].
     pub fn reset(&mut self) {
         self.inner.reset()
@@ -246,6 +262,15 @@ impl Vm {
         Ok(returns)
     }

+    pub fn run_function(
+        &self,
+        func_name: impl AsRef<str>,
+        params: impl IntoIterator<Item = sys::WasmValue>,
+    ) -> WasmEdgeResult<Vec<sys::WasmValue>> {
+        self.inner.run_function(func_name.as_ref(), params)
+    }
+
+
     /// Returns the type of a WASM function.
     ///
     /// # Arguments

As the API design principles of wasmedge-sdk and wasmedge-sys are different, the wasmedge-bindgen should provide supports for them, respectively. It is not recommended that refactoring the existing APIs of wasmedge-sdk and wasmedge-sys to simplify the design of wasmedge-bindgen. I strongly recommend that wasmedge-bindgen should focus on the support for wasmedge-sdk, because wasmedge-sys is a low-level layer in the architecture of WasmEdge Rust bindings, we should help the early-phase end users migrate their projects onto wasmedge-sdk. Therefore, it would be much better to provide full support for wasmedge-sdk and weaken the support for wasmedge-sys gradually.

Could we make wasmedge-bindgen as a part of wasmedge-sdk? I'm not sure why wasmedge-bindgen need to be a standalone project.

Maybe related to https://github.com/WasmEdge/WasmEdge/issues/1612.

Per my understanding, wasmedge-sdk will expose much less APIs to user and will provide more builtin senarios support, right?

apepkuss commented 2 years ago

Hi @chenyukang

Q: Could we make wasmedge-bindgen as a part of wasmedge-sdk? I'm not sure why wasmedge-bindgen needs to be a standalone project.

As I know, wasmedge-bindgen is derived from wasm-bindgen, which is not part of WasmEdge. This is the primary reason.

Here are other considerations. According to the current phase of WasmEdge Rust bindings (including wasmedge-sdk and its dependencies), the APIs of wasmedge-sdk is still not stable, as there are still some new WebAssembly proposals and features under development, which could introduce some changes in C-APIs, such as the fix of WasmEdge issue #1659; in addition, wasmedge-sdk is in the first version and we believe there are still space for improvements on APIs and even the architecture, and we will do it based on the feedback from the community. Therefore, it would be better to keep Rust bindings as simple as possible, so that we can obtain the flexibility of how to support complex interface types, wasmedge-bindgen or runtime-native interface types.

Q: wasmedge-sdk will expose much less APIs to user and will provide more builtin senarios support, right?

Basically, the first design principle of wasmedge-sdk is focusing on ergonomics but not "fewer APIs". We hope that the high-level APIs can provide a programming pattern Rust developers like, for example, chaining, builder pattern, etc. As the statements in the answer to the first question, wasmedge-sdk is still at the beginning stage. We hope Rust developers like it, but honestly, we believe there is space for improvements. In addition, we also would like to verify if the APIs in the current version of wasmedge-sdk would cover the requirements that end users need in their projects. That's also a factor that we view as a kind of "feedback" for our future improvements.

chenyukang commented 2 years ago

@apepkuss Thanks for answering.

As for the design principle of wasmedge-sdk, I think the most important thing is "less suprise" and convinent for programmers.

There are some programming patterns in Rust programming, chaining is good, but I think builder pattern maybe is not a must one.

The last time I was confused on this is at 😂 https://github.com/containers/oci-spec-rs/pull/49 https://github.com/containers/oci-spec-rs/issues/60

You see, different people have different preference.

I saw you published new version(stable version?) for wasmedge-sdk, I will polish my changes after updating the latest wasmedge-sdk.

And also will try to replace wasmedge-sys with wasmedge-sdk in https://github.com/second-state/dapr-wasm

apepkuss commented 2 years ago

Hi @chenyukang

Thanks for your feedback. I totally agree with the point of "less surprise and convenient". We'll put your feedback into our feedback pool so that we can make improvements to our APIs in future releases.

For the question about the stability of the current APIs in wasmedge-sdk, according to our dev plan, runtime will introduce some new features in the next release (which could be v0.11.0), which should bring about some breaking changes on the current APIs. Therefore, my suggestion is that you can put off the migration until the new version is released.

Thanks for your feedback and question again :)

chenyukang commented 2 years ago

@apepkuss I'm trying to make the test project to work, seems the method init_wasi is lack in WasiInstance.

The Rust SDK return inner wasi module:

pub fn wasi_module(&mut self) -> WasmEdgeResult<WasiInstance> {
        let inner_wasi_module = self.inner.wasi_module_mut()?;

        Ok(WasiInstance {
            inner: inner_wasi_module,
        })
    }

In the test code, what I want to do is:

image

init_wasi don't have a public method, and inner is private.

chenyukang commented 2 years ago

I will follow the principle that don't make any more changes on your Rust SDK, and make wasmedge-bindgen to work.

apepkuss commented 2 years ago

@apepkuss I'm trying to make the test project to work, seems the method init_wasi is lack in WasiInstance.

The Rust SDK return inner wasi module:

pub fn wasi_module(&mut self) -> WasmEdgeResult<WasiInstance> {
        let inner_wasi_module = self.inner.wasi_module_mut()?;

        Ok(WasiInstance {
            inner: inner_wasi_module,
        })
    }

In the test code, what I want to do is:

image

init_wasi don't have a public method, and inner is private.

@chenyukang I found that the version of wasmedge-sdk you're using is 0.1.0, right? Could you please update it to the latest release 0.3.0? Then, you can use the initialize method to do the initialization. Thanks!

chenyukang commented 2 years ago

@apepkuss Thanks, I have solved this issue and move on.

Here is another issue, in previous wasmedge-sys's Bindgen, here we call vm.instantiate, https://github.com/second-state/wasmedge-bindgen/blob/6f1fc8aa79b4d0a2211f0847583f33a60d69b93b/host/rust/src/lib.rs#L320

VM in Rust SDK also didn't exported this function, how do I call the API, which from the inner VM (which is from wasmedge-sys).

In my local repo, I need to wrap this:

+    pub fn instantiate(&mut self) -> WasmEdgeResult<()> {
+        self.inner.instantiate()
+    }

In the test code, we use it like this:

    let mut bg = Bindgen::new(vm);
    bg.instantiate();
apepkuss commented 2 years ago

@chenyukang Yes, you're right. wasmedge-sdk does not expose the wasmedge_sys::Vm::instantiate method, the reason is that this method is only used to register active wasm modules (also called anonymous modules) in the context of wasmedge_sys, so in the context of wasmedge-sdk we abstract the design and "wrap" all the methods related to the registration of active wasm modules into the private method wasmedge_sdk::Vm::register_active_module which is then used by the public method wasmedge_sdk::Vm::register_module. This is why you cannot find a "instantiate" method in wasmedge-sdk::Vm. I'm not familiar with wasmedge-bindgen, is it possible for wasmedge-bindgen to use a workaround to solve the issue?