-
Notifications
You must be signed in to change notification settings - Fork 225
Draft: Symmetric ABI tracking and minor side functionality #1098
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Thanks for this! Personally though I'd probably leave it mostly up to you as to how best to chunk up this work and land it. The 0.3 bits may take a bit to land so if you can avoid depending on that it might make sense, although if it is likely to heavily conflict with @dicej's work we might want to strategize in the near term. Otherwise though I find it difficult to predict how to split up "unknown code" in the sense that I haven't reviewed this yet so having an opinion on how best to chunk it up would need a review/understanding on my behalf first, so this is also why I'll trust to you break it up since that makes the review load easier too :) |
Is "symmetric C++ API" related to the symmmetric ABI? If so, then see my comments below about the symmetric ABI. And for the async parts, see my comments below about async :-).
As discussed in #395, host support no longer lives in the wit-bindgen repository, so this would make sense to split out, either into wamr itself or a separate repo somewhere. See this section for a discussion of what other hosts are doing.
What would you think about factoring these changes out to live in their own repository? Native ABI interfaces sound like they have some similar needs to cool projects like wRPC, wasm-http-tools, and hopefully more such tools coming, which live in separate repositories, sharing factored-out building blocks like wit-parser.
I think the main thing to do here rhg tnow is to just avoid creating more work for Joel, which I expect means waiting on any changes that would depend on these parts for now.
This would be useful to a lot of people, so if you're looking for pieces to prioritize here, this is my suggestion :-). |
Yes and no, https://github.com/cpetig/wit-bindgen/blob/main/crates/cpp/DESIGN.md will give you a good idea why the old API is really horrible, although the new API introduces some allocation for canonical as it is tailored towards zero allocations for the symmetric ABI.
I agree, that other host parts are in a separate repo, but most of the code is shared between the two C++ codegens, and I really want to avoid creating more forks of my code. I will think of a viable way.
Right now it is just another command line switch for Rust and C++ to support the symmetric ABI, and as both bindgens are meant to be a drop-in replacement depending on which ABI you target, I think having the code in one place makes sense.
Right now I experiment with streams for symmetric, async calls already work well (no codegen yet). I plan to separate this into parts after #1082 is merged (less work for me, less chance for conflicts).
Yes, but this is the most advanced part, see also the discussion on zulip about zero-copy and shared memory which is identical to this change, |
Signed-off-by: Christof Petig <[email protected]>
…into cpp-bindgen
Update: Also the alien move semantic was a misunderstanding between me and the C++ standard. @TartanLlama explained the details and (move-) passing by value will become the new standard. This branch contains a symmetric ABI which still avoids allocation overhead using the symmetric API (passing arguments by reference in both directions). |
My fork has aggregated several potentially separable parts over the past years.
I would like to come up with a plan to incrementally merge in interest order, so I would love to get feed-back about which features are most wanted/urgent to separate them in the most efficient way. Perhaps we want to merge Joel's work first to minimize friction on his work/merge.
Some of these features are unfinished but given enough interest I clearly want to make them upstreamable.