dylibso / chicory

Native JVM WebAssembly runtime
Apache License 2.0
319 stars 27 forks source link

Host Functions not very composable #357

Open bhelx opened 1 month ago

bhelx commented 1 month ago

I've had this issue where working with host functions and imports can be awkward. For example. if i want to support both wasi and some custom host functions, i need to compose them like this:

HostFunction[] extra = createMyCustomHostFunctions();
HostFunction[]  wasiFuncs = wasi.toHostFunctions();
HostFunction[] allImports = new HostFunction[wasiFuncs.length + extra.length];
System.arraycopy(wasiFuncs, 0, allImports, 0, wasiFuncs.length);
System.arraycopy(extra, 0, allImports, wasiFuncs.length, extra.length);

var imports = new HostImports(allImports);

I can think of a number of ways to address this. A quick fix might be to allow the HostImports constructor to take an arraylist. then we turn it into an array in the constructor. Or there may be a better interface to build and compose these that we should consider. Now might be a good time to create a builder interface for host modules. I think wazero has a decent interface for this: https://github.com/tetratelabs/wazero/blob/main/examples/import-go/age-calculator.go#L41 but of course we could do it in a more idomatic Java way. I can easily do the first solution but I felt I'd give an opportunity for some of the experienced Java developers on the team to bring their opinion on a good interface.

andreaTP commented 1 month ago

I think that having Builder interface for HostImports that allows for adding multiple imports is a good solution aligned with what we are building in the rest of the codebase:

HostImports.builder().addFunctions(wasiFuncs).addFunctions(myCustomFunctions).build();

For context, I proposed something similar in #159 but I would be happy to see it finalized!

bhelx commented 1 month ago

Yes I think this makes sense. It should certainly be a builder. I'm not sure if we should allow more control like the wazero api seems to. perhaps @evacchi could provide some input.

evacchi commented 1 month ago

the wazero api includes a concept of "host modules", which are namespaced groupings of functions. The low-level concept of module imports is not exposed to the end user, they instead "instantiate" one or more host modules.'

andreaTP commented 1 month ago

I'm not sure if we should allow more control like the wazero api seems to.

The builders for HostFunctions are fine, but I would not spend much effort there, they are still going to be pretty low level even if a little nicer to type. Having some kind of "bindgen" to generate the boilerplate out of the Module Type signatures is much more appealing to me.

bhelx commented 1 month ago

@evacchi do you think it scales to just have a list of functions like we do? Do you think we need to model the concept of a module? The flat list of functions seems to work for the time being, but i'm unsure if there will eventually be some things we need to do on a higher level or there is a reason that wazero has modeled it that way. I think some of this might be best discussed when we talk about the 1.0 API but thought i'd get your perspective.

evacchi commented 1 month ago

I think host modules in wazero are not just a convenience, they also have significance in how functions are invoked cross-modules (e.g. how native code is being generated to support the host calls).

In general, having functions grouped in modules instead of freestanding should be better to manage their lifecycle; e.g. these functions can be anything closures, etc. If you destroy a host module you should be sure that everything it references can be freed.

So long story short, for the time being this is fine, but IMO it's possible that adding a concept of a host module might be convenient and also useful internally.

bhelx commented 1 month ago

So long story short, for the time being this is fine, but IMO it's possible that adding a concept of a host module might be convenient and also useful internally.

okay yeah that was my intuition but i wasn't sure. Since that is the case I think I should save this for when i design the 1.0 API.

andreaTP commented 1 month ago

@bhelx looking forward to it, do you already have an idea on how will look like from the user perspective?

bhelx commented 1 month ago

Yes, but it's just a list of things in my head and stuff to run down that I've noticed in other runtimes. Will start to write them up in an issue and we can discuss this summer.