wasmtime and wasmer together have issues when linking
JanFalkin opened this issue · 13 comments
The problem stems from the fact that both wasm runtimes (wasmer and wasmtime) export some of the same symbols into the flat c namespace. For example, wasm_config_new is exported by both runtimes. So, when wapc links, which one does it get? I noticed this when trying to integrate wasmer's engine metering and being confounded by wasm_config_new not calling the wasmer export. I'm not sure how to work around this as there are numerous symbols to account for.
Commenting to help save time as I lost hours on this over the weekend.
I attempted to do this and spent several hours trying in wazero's benchmarks. It is triggered when using WASI. I was unable to work it out and ended up having to separate them out into different go.mod files. PS A mixed go.mod also caused a panic in WasmEdge in a different way, which resolved as well when isolated. I know WasmEdge isn't done here, but it was interested as while wasmtime-go and Wasmer-go are nearly identical wasmedge is not.
https://github.com/tetratelabs/wazero/tree/main/internal/integration_test/vs
cc @wuhuizuo in case you figured a way out of this. Note again, I didn't have conflict until I started using WASI in both wasmedge and wasmtime for reasons @JanFalkin mentioned. https://github.com/wuhuizuo/go-wasm-go
@JanFalkin ps I'm not an end-user of either wapc-go or wasmtime, Wasmer, so me doing this is less legit. As an end user, here's my suggestion:
While wasmer is >v1.0, wasmtime-go is not. You could ask wasmtime to change their symbols so that they don't collide with Wasmer. While API familiarity may have been intentional between wasmtime and Wasmer, this sort of thing was likely an accident, and they might fix it. If any time, it is more likely to fix before they do version 1.0
my 2p
@codefromthecrypt I will try to attend the next wasmtime meeting and see if I can elicit a discussion. It's actually the respective c-apis where the naming becomes an issue. I can somewhat understand as they (wasmtime and wasmer) were developed separately and some of the naming that was chosen seems reasonable IMHO. It's just they don't blend well together in cgo. I will update this issue as I move to some resolution.
@JanFalkin good point yeah this will be in the underlying projects, and possibly effect other bridges (Java's JNI?). Luckily the version of wasmtime's core lib is still <1.0 so might have luck. If were me, I would be inclined to change my C api for this reason alone, as it would help people transition to my stuff. Big Bang transitions are annoying.
last spam on this.. I was wrong assuming >1.0 meant things can't change as a lot of the API surface area changed at least in the wasmer-go project after 1.0. It doesn't matter who changes, and either might do it.
EOF!
I got more info. Wasmtime and Wasmer are both downstream of this https://github.com/WebAssembly/wasm-c-api. So, I don't think they can ever really live together in the same process space without unique symbols being exposed along with the legacy. IMHO truly ugly. We may want to consider using go build tags in wapc-go to separate the engines?
I haven't noticed anyone coming in on this, so I guess the options are
- go.mod files for projects using CGO
- build tags for projects using CGO
no on go.mod
I suppose go.mod files would feel unfair as that would make wazero the default and also invalidate the README. The README should probably still prefer something else until we cut a version tag, and even after we cut something else, it should be left alone until someone not working on wazero wants it to change.
yes on build flags
wasmer and wasmtime both handle the shared libs as a part of release assets, so we can treat this in two ways:
- opt in via build tag
- opt out via build tag
I think probably opt-out would be best as it would allow the project to continue to build (ex people can fix unit tests or refactor without drift). Either way, users have to specify a build tag, so this is fine I think, right?
I went back and looked at the project assets. It looks like wasmtime is nearly across the main platforms (notably no aarch64 windows, though that's an obscure platform).
wasmer:
//go:build (amd64 || arm64) && !windows && cgo && !wasmtime
wasmtime:
//go:build (((amd64 || arm64) && !windows) || (amd64 && windows)) && cgo && !wasmer
I can offer to help raise this PR if someone else doesn't want to. I think it is good for folks to contribute outside their area of interest.
@codefromthecrypt Sorry for delayed response. I have already implemented similar but was defaulting to wasmer in our case as it provides runtime metering where wasmtime does not. That is our production case. Wasmtime does provide debugging facilities for the wasm bitcode which is invaluable but not our main case. Make sense? I can certainly work around the default being wasmtime, so if you want to raise a pull, I would be a fan.
cool probably I added distraction about which is default anyway :) I'll raise a PR in a bit as I'm back online
@JanFalkin I realized that the examples had drifted and were confusingly using different engines depending on README vs the source. I went ahead and used wazero and added tests that the example can at least execute (via github) PTAL #30
@codefromthecrypt I had a quick look over #30 and it looks good. Funny the example uses wazero, as our Go architect asked if the solution supported it :-). You can close this issue if you like when you have pulled in. Thanks!
I had a quick look over #30 and it looks good. Funny the example uses wazero, as our Go architect asked if the solution supported it :-). You can close this issue if you like when you have pulled in. Thanks!
Cool. We implement features on-demand, so if something is "missing" maybe needs asking for ;) See you around!