Milek7 opened PR #7940 from Milek7:mt-c-api
to bytecodealliance:main
:
I'm not sure how to handle SharedMemory here. Possibly it could use Extern interfaces, but I had some trouble with it. Extern seem to assume that it's associated with Store, but shared memories aren't. Instead I added dedicated
define_sharedmemory
in linker. Is this acceptable or I need to fight with Extern more to get it to work?
Milek7 requested alexcrichton for a review on PR #7940.
Milek7 requested wasmtime-core-reviewers for a review on PR #7940.
Milek7 updated PR #7940.
Milek7 edited PR #7940:
I'm not sure how to handle SharedMemory here. Possibly it could use Extern interfaces, but I had some trouble with it. Extern seem to assume that it's associated with Store, but shared memories aren't. Instead I added dedicated
wasmtime_linker_define_sharedmemory
. Is this acceptable or I need to fight with Extern more to get it to work?
pchickey commented on PR #7940:
From the context here I'm not sure why the wasmtime_wasictx methods are required to use threads. I'm planning a renovation which will swap out the c api's wasi implementation from
wasi-common
towasmtime-wasi::preview2
, which will shortly be [promoted to the root ofwasmtime-wasi
)(https://github.com/bytecodealliance/wasmtime/pull/7933), and which is not compatible with wasmtime-wasi-threads.
pchickey edited a comment on PR #7940:
From the context here I'm not sure why the wasmtime_wasictx methods are required to use threads. I'm planning a renovation which will swap out the c api's wasi implementation from
wasi-common
towasmtime-wasi::preview2
, which will shortly be [promoted to the root ofwasmtime-wasi
)(https://github.com/bytecodealliance/wasmtime/pull/7933), and which is not compatible withwasmtime-wasi-threads
.
pchickey edited a comment on PR #7940:
From the context here I'm not sure why the wasmtime_wasictx methods are required to use threads. I'm planning a renovation which will swap out the c api's wasi implementation from
wasi-common
towasmtime-wasi::preview2
, which will shortly be promoted to the root ofwasmtime-wasi
, and which is not compatible withwasmtime-wasi-threads
.
alexcrichton commented on PR #7940:
I think adding bits and functions to manage shared memories in the C API would be great to have, but I would agree with @pchickey that the WASI bits may want to be left out for now. I would echo his same sentiments in terms of it's not clear what to do with the implementation of WASI and threads in the wasmtime repo into the future and I think it would be best to not add new dependencies on something we're hoping to remove (aka
wasi-common
)
wasictx constructor is needed to attach the same WASI context to multiple instances (threads). If preview2 does not support threads that's somewhat of a problem for my use-case :(
I implement thread spawn function on C side instead of usingwasmtime-wasi-threads
, but I don't think that makes any difference
github-actions[bot] commented on PR #7940:
Subscribe to Label Action
cc @peterhuene
<details>
This issue or pull request has been labeled: "wasmtime:c-api"Thus the following users have been cc'd because of the following labels:
- peterhuene: wasmtime:c-api
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
@Milek7, you may be interested in https://github.com/WebAssembly/component-model/pull/291. Let's talk more tomorrow on Zulip; maybe you want to help out with implementing some of those bits?
alexcrichton commented on PR #7940:
That makes sense that wasi contexts can't be shared today, but this API being added is exposing a contract where a wasi context can be attached to multiple stores which we may not wish to guarantee. For example thisi enables having one long-lived wasi context which is shared over its lifetime by many instances. While a perhaps interesting use case that's not necessarily something I think we want to commit to just yet.
Milek7 updated PR #7940.
Dropped wasictx functions.
alexcrichton commented on PR #7940:
Ok, thanks! Can you detail more how come this couldn't be folded into
wasmtime_extern_t
? For example with this support you can't extract a shared memory export or inspect the import/export types for example.
I was having some trouble with it, but looking at it now I might have confused wasmtime_extern_t with wasm_extern_t, ouch. I will take another look next week.
alexcrichton commented on PR #7940:
Ah ok makes sense! In that case yeah I'd recommend trying to get this integrated with the
wasmtime_*
variant. If that has issues though I can try to help work through them as they come up
Milek7 updated PR #7940.
Milek7 updated PR #7940.
Milek7 updated PR #7940.
Done
alexcrichton submitted PR review:
Thanks!
Milek7 updated PR #7940.
alexcrichton has enabled auto merge for PR #7940.
alexcrichton merged PR #7940.
Last updated: Nov 22 2024 at 16:03 UTC