bytecodealliance / wasmtime-dotnet

.NET embedding of Wasmtime https://bytecodealliance.github.io/wasmtime-dotnet/
Apache License 2.0
411 stars 52 forks source link

Question: Thread-Safety #90

Open christophwille opened 2 years ago

christophwille commented 2 years ago

I have asked that in the past most likely via an at-mention in https://github.com/christophwille/dotnet-opa-wasm/ - but today it came back up for multi-threaded usage of the Wasmtime API. I have cooked up a sample and I am pretty sure I remember our past discussion wrongly... the overall goal is to not incur the compilation hit.

https://github.com/christophwille/dotnet-opa-wasm/blob/master/spikes/HigherLevelApisSpike/IPolicyFactory.cs#L129

I load a wasm file into a byte array, create an engine and use the engine for module load - and for the sake of discussion, throw away that engine. The question now is: is Module safe to use in parallel later for

// _store is created on a new Engine, _linker same var instance = _linker.Instantiate(_store, module);

Ie can one cached Module be used to create unlimited instances in parallel.

Thanks.

peterhuene commented 2 years ago

Yes, you should be able to share a module between threads for the purpose of instantiation. The module will keep a reference on the engine used to create it, but you can dispose of your own reference to that engine if it is no longer needed.

It's only Store (and all store-related objects like instances) that cannot be shared between threads.

Also, the .NET API supports loading of modules from "precompiled" modules, but not creating them as we don't currently expose the Engine::precompile_module method from the Wasmtime API for the .NET API to use. If you have Wasmtime installed, you can use wasmtime compile <module> to generate a precompiled file (cwasm), however.

The benefit of using a precompiled file is that there is no JIT compilation at all; the module is simply mapped into memory upon load.

christophwille commented 2 years ago

Ah, didn't know about the precompilation feature, interesting though (very much so).

Then my current object model was already correctly split between Module and instance with linker and store. And for my new approach I only need to cache one Module for creating multiple instances.

Edited for completeness:

using var opaRuntimeForLoad = new OpaRuntime();
using var module = opaRuntimeForLoad.Load("example.wasm");

using var opaRuntime = new OpaRuntime();
using var opaPolicy = new OpaPolicy(opaRuntime, module);
using var opaPolicy2 = new OpaPolicy(opaRuntime, module);
peterhuene commented 2 years ago

From a cursory inspection, that should work as you intend.

willhausman commented 2 years ago

Yes, you should be able to share a module between threads for the purpose of instantiation. The module will keep a reference on the engine used to create it, but you can dispose of your own reference to that engine if it is no longer needed.

@peterhuene does this mean that it doesn't matter if the same Engine is used to create an Instance from a Module?

e.g.

var engine = new Engine();
var module = Module.FromFile(engine, file);

// now stuff happens with the application keeping module in memory but disposing engine

var engine = new Engine();
var linker = new Linker(engine);
var store = new Store(engine);
DoSetupLinkerStuff();
var instance = linker.Instantiate(store, module);  // <- different engines
peterhuene commented 2 years ago

Cross-engine instantiation isn't supported and I'd expect that to throw an exception (at least it should; please file a bug if not).

You'll need to use the same engine for the module, linker, and store. I forgot that we don't have an Engine property on Module or Linker in the .NET API like we do in the underlying Wasmtime API in Rust as it's not exposed via the C bindings.

You'll need to keep a reference to the same engine around for now.

christophwille commented 2 years ago

You'll need to use the same engine for the module, linker, and store. I forgot that we don't have an Engine property on Module or Linker in the .NET API like we do in the underlying Wasmtime API in Rust as it's not exposed via the C bindings.

Could you expose such a property? Or is this a no-no for the .NET object model? Because with that would a allow

using var opaPolicy = new OpaPolicy(module.Runtime, module);

without keeping the Engine around "some other way". The Engine is thread-safe though?

christophwille commented 2 years ago

Cross-engine instantiation isn't supported and I'd expect that to throw an exception (at least it should; please file a bug if not).

https://github.com/christophwille/dotnet-opa-wasm/blob/master/src/Opa.Wasm.UnitTests/ThreadSafetyTests.cs#L10 it does fail but not very "obvious" as to the cause unless you look at the line it is failing on https://github.com/christophwille/dotnet-opa-wasm/blob/master/src/Opa.Wasm/OpaPolicy.cs#L140 and know the innards of Wasmtime.

christophwille commented 2 years ago

Kind of related question: if Engine and Module are tied tightly together - is there somewhere guidance on tradeoffs/use cases for (a) one engine married to one module (possibly hundreds around at the same time) vs (b) one engine for multiple modules? Which approach is "better", to use when?

peterhuene commented 2 years ago

Right now there isn't a way for the .NET API to expose an Engine property on either Module or Linker without storing a reference on the .NET side which is generally something I've been trying to avoid doing (I prefer the native side keep the references to keep things simpler and relatively deterministic).

It wouldn't be hard to add C API functions to get a reference to the engine for a given module or linker and then make use of those from the .NET API, though. I can look at doing that when I get a chance.

With regards to the exception you're getting, that's definitely an unexpected and confusing one, so I can dig into why it isn't the expected "inter-engine instantiation isn't supported" message.

peterhuene commented 2 years ago

Oh and Engine is thread safe too.

peterhuene commented 2 years ago

It's generally fine to create a single Engine per process and create all modules from that engine.

An engine is simply a JIT compiler, an instance allocator, and a signature registry.

The JIT compiler is thread-safe and will synchronize the compilation of any modules being compiled for that engine.

The instance allocator is what is used to allocate new instances from the instantiation of modules. By default (and what's supported by the .NET API), it's an on-demand allocator that simply allocates the needed resources of an instance when requested; it doesn't store any state that grows over time. The Rust API has another allocator type that uses resource pools to allocate from, but that's not yet supported by .NET.

Lastly, the signature registry is a global place we assign unique ids to function signatures. This is used as an optimization so that we can very quickly verify function signatures when doing indirect calls from WebAssembly. This will grow over time, but is bounded by how many unique function signatures are encountered in compiled modules and host functions (thus likely a fairly small set).

None of these things would be impacted from having a single Engine shared across all modules.