lunatic-solutions / lunatic

Lunatic is an Erlang-inspired runtime for WebAssembly
https://lunatic.solutions
Apache License 2.0
4.62k stars 138 forks source link

Shared compiled wasm modules #126

Closed tqwewe closed 1 year ago

tqwewe commented 2 years ago

Allows compiled wasm modules to be shared between the same node.

Two new host functions were added:

Modules are no longer pre-linked. The upside of this is that WasmtimeCompiledModule no longer has a generic (which removes generics from many other places too), but the downside is that it might be a little less performant.

I'll try to get some benchmarks done to test this PR's performance compared to before.

tqwewe commented 2 years ago

Main branch

spawn process           time:   [107.77 µs 108.37 µs 109.00 µs]                          
                        change: [-71.703% -71.432% -71.123%] (p = 0.00 < 0.05)
                        Performance has improved.

This PR

spawn process           time:   [384.67 µs 385.86 µs 387.08 µs]                          
                        change: [+251.58% +255.12% +258.40%] (p = 0.00 < 0.05)
                        Performance has regressed.

Seems like the performance does decrease substantially sadly :(

I'm not sure how to get past this since with pre linking requires WasmtimeCompiledModule to have a generic, which causes issues when sharing the module across processes.

bkolobara commented 2 years ago

We only have one state (generic T) though. Could we bubble it up all the way to Resource<T> and then use the state type to create the specific Resource? I didn't look at the code for some time now and am just assuming bunch of stuff.

tqwewe commented 2 years ago

I'm not entirelly sure what you mean, but its possible you might have a better solution. Optimally we can share wasm compiled modules across processes while keeping the same performance with pre-linking.

bkolobara commented 2 years ago

I will take a look at it and see if there is a way to make it work.

tqwewe commented 2 years ago

After re-reading your comment, I think I understand. The problem with Resource having a generic T is that we put Resource in a vec to store all resources.

pub struct DataMessage {
    // ...
    pub resources: Vec<Resource>,
}

If Resource had a generic here, then we couldnt store a vec of resources with different types.

Though, this might be fine to require every resource to have the same T state actually?

I've just made a branch on my fork which has the same performance and allows compiled modules to be shared via Arc by simply doing Vec<Resource<T>> (each resource has the same type). https://github.com/tqwewe/lunatic/tree/feat/shared-module-2

If you'd like, I can open it as a separate PR and close this one.

tqwewe commented 2 years ago

The main requirement of the state is that it implements ProcessState. It seems like there's really only one type that implements ProcessState, and that is DefaultProcessState. Could we remove ProcessState trait and rename DefaultProcessState to ProcessState? So ProcessState would be a sturct instead of a trait.

bkolobara commented 2 years ago

The main requirement of the state is that it implements ProcessState. It seems like there's really only one type that implements ProcessState, and that is DefaultProcessState. Could we remove ProcessState trait and rename DefaultProcessState to ProcessState? So ProcessState would be a sturct instead of a trait.

The reason why we introduced the ProcessState trait is that in a case where we embed the lunatic_runtime inside of another rust project, you can provide your own process state and don't need to use all the features.

For example, if you don't need networking in your embedded usage, you just provide a ProcessState without the networking parts and don't implement NetworkingCtx for it. So you can still use all the other built in host functions that we provide, except the networking ones.

At the moment there is only one implementation of the ProcessState (DefaultProcessState), but this could change in the future. Even for "per feature (networking, timeouts, spawning, ...) testing purposes", we could provide smaller process states that are crate local. We would not be able to use the DefaultProcessState because of circular dependencies in that case.

tqwewe commented 1 year ago

Closing in favor of #148