Wasmtime GitHub notifications bot (Feb 14 2024 at 18:53): Wasmtime GitHub notifications bot (Feb 14 2024 at 18:53):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_sharedmemoryin linker. Is this acceptable or I need to fight with Extern more to get it to work?
Wasmtime GitHub notifications bot (Feb 14 2024 at 18:53):Milek7 requested alexcrichton for a review on PR #7940.
Wasmtime GitHub notifications bot (Feb 14 2024 at 18:53):Milek7 requested wasmtime-core-reviewers for a review on PR #7940.
Wasmtime GitHub notifications bot (Feb 14 2024 at 18:59):Milek7 updated PR #7940.
Wasmtime GitHub notifications bot (Feb 14 2024 at 19:03):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?
Wasmtime GitHub notifications bot (Feb 14 2024 at 21:43):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-commontowasmtime-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.
Wasmtime GitHub notifications bot (Feb 14 2024 at 21:43):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-commontowasmtime-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.
Wasmtime GitHub notifications bot (Feb 14 2024 at 21:56):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-commontowasmtime-wasi::preview2, which will shortly be promoted to the root ofwasmtime-wasi, and which is not compatible withwasmtime-wasi-threads.
Wasmtime GitHub notifications bot (Feb 14 2024 at 22:37):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)
Wasmtime GitHub notifications bot (Feb 14 2024 at 22:39):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
Wasmtime GitHub notifications bot (Feb 15 2024 at 01:03):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>
Wasmtime GitHub notifications bot (Feb 15 2024 at 02:36):@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?
Wasmtime GitHub notifications bot (Feb 15 2024 at 19:01):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.
Wasmtime GitHub notifications bot (Feb 15 2024 at 19:56):Milek7 updated PR #7940.
Wasmtime GitHub notifications bot (Feb 15 2024 at 23:58):Dropped wasictx functions.
Wasmtime GitHub notifications bot (Feb 16 2024 at 16:09):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.
Wasmtime GitHub notifications bot (Feb 16 2024 at 16:34):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.
Wasmtime GitHub notifications bot (Feb 16 2024 at 16:57):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
Wasmtime GitHub notifications bot (Mar 02 2024 at 00:48):Milek7 updated PR #7940.
Wasmtime GitHub notifications bot (Mar 02 2024 at 01:01):Milek7 updated PR #7940.
Wasmtime GitHub notifications bot (Mar 02 2024 at 01:33):Milek7 updated PR #7940.
Wasmtime GitHub notifications bot (Mar 02 2024 at 02:28):Done
Wasmtime GitHub notifications bot (Mar 07 2024 at 17:34):alexcrichton submitted PR review:
Thanks!
Wasmtime GitHub notifications bot (Mar 08 2024 at 01:35):Milek7 updated PR #7940.
Wasmtime GitHub notifications bot (Mar 08 2024 at 01:39):alexcrichton has enabled auto merge for PR #7940.
Wasmtime GitHub notifications bot (Mar 08 2024 at 02:20):alexcrichton merged PR #7940.
Last updated: Dec 23 2024 at 12:05 UTC