-
Notifications
You must be signed in to change notification settings - Fork 22
Experimental Host Function Support (and rc4 release) #39
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
Conversation
| .as_ident() | ||
| .unwrap() | ||
| .sym; | ||
| Param::new(vn, typ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sure there is a nicer way to pull out all these values with some high level rust magic but this was what came naturally to me.
crates/core/src/globals.rs
Outdated
| let mem_obj = context.object_value()?; | ||
| mem_obj.set_property("fromBuffer", memory_from_buffer)?; | ||
| mem_obj.set_property("find", memory_find)?; | ||
| mem_obj.set_property("readBytes", read_bytes)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going tot think about this interface a little bit more
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: I think we should add a free as well. Need to reference the other PDK APIs and try to match it.
| let data = args.get(0).ok_or(anyhow!("Expected data argument"))?; | ||
| if !data.is_array_buffer() { | ||
| bail!("Expected data to be an array buffer"); | ||
| } | ||
| let data = data.as_bytes()?; | ||
| let m = extism_pdk::Memory::from_bytes(data)?; | ||
| let mut mem = HashMap::new(); | ||
| let offset = JSValue::Int(m.offset() as i32); | ||
| let len = JSValue::Int(m.len() as i32); | ||
| mem.insert("offset".to_string(), offset); | ||
| mem.insert("len".to_string(), len); | ||
| Ok(JSValue::Object(mem)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might move some more of this logic up into the Javascript shim layer. Thinking about it. Won't affect the interface though.
| @@ -0,0 +1,10 @@ | |||
| declare module 'main' { | |||
| export function greet(): I32; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exports must have this signature
| myHostFunction1(ptr: I64): I64; | ||
| myHostFunction2(ptr: i64): i64; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imports must have this signature, for now
| let msg = "Hello from js 1" | ||
| let mem = Memory.fromBuffer((new TextEncoder().encode(msg)).buffer) | ||
| let ptr = myHostFunction1(mem.offset) | ||
| let response = new TextDecoder().decode(Memory.readBytes(ptr)) | ||
| if (response != "myHostFunction1: " + msg) { | ||
| throw Error(`wrong message came back from myHostFunction1: ${response}`) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going to add some wrappers on Memory to make this a little nicer
| // let tmp_dir = "/tmp/derp"; | ||
| // let core_path = PathBuf::from("/tmp/derp/core.wasm"); | ||
| // let export_shim_path = PathBuf::from("/tmp/derp/export-shim.wasm"); | ||
| // let import_shim_path = PathBuf::from("/tmp/derp/import-shim.wasm"); | ||
| // let linked_shim_path = PathBuf::from("/tmp/derp/linked.wasm"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had this for testing. will clean up or add a command flag for this.
| bail!("wasm-merge failed. Couldn't merge export shim"); | ||
| } | ||
|
|
||
| remove_dir_all(tmp_dir)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't realize that the temp dir gets deleted when the object gets dropped.
| .arg("--enable-reference-types") | ||
| .arg("--enable-bulk-memory") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: I need to verify this is going to work in non wasmtime runtimes
| .arg(&opts.output) | ||
| .arg(&linked_shim_path) | ||
| .spawn()?; | ||
| let status = command.wait()?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: need a better error message for when wasm-merge can't be found on the machine
|
Going to merge and get rc4 out now. Will be back next week with follow ups! |
This builds on my last PR and adds support for Host Functions. Currently the limitation is that host functions must have 1 i64 ptr param and 1 i64 ptr result. We enforce this through validation of the typescript interface file.
To enable host functions you just need to declare another interface
extism:host/user:These needs to have this signature for now. We will work on making different function arities work next.
To use these you need to use
Host.getFunctions():Calling them is a similar process to other PDKs. You need to manage the memory with the
Memoryobject and pass across an offset as the i64 ptr. Using the return value means dereferencing the returned i64 ptr from Memory.See example in examples/host_funcs