Skip to content
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

Add an 'async' hint to functions #442

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add an 'async' hint to functions #442

wants to merge 1 commit into from

Conversation

lukewagner
Copy link
Member

This PR adds non-blocking to functype (both in WAT and WIT). It's a pretty small addition and doesn't touch validation/runtime.

While it initially seemed attractive to enforce non-blocking with trap-on-block semantics, there are valid scenarios where a callee might actually want/need to block and where the loss of concurrency in the caller is fine. Thus the PR proposes making non-blocking just a hint (ignored by validation/runtime) to inform bindings generation (e.g., allowing bindings generators that make all functions async by default emit non-async functions for non-blocking).

constructor implies non-blocking (since new expressions in most languages can't be async). However, to avoid breaking wasip2, we don't (yet) require validation of [constructor]-named functions to contain non-blocking (adding this to the list of warts to remove in the next breaking change). In any case, bindings generators can always just take the non-blocking hint directly from seeing [constructor].

@lukewagner lukewagner force-pushed the non-blocking branch 2 times, most recently from c8be8a4 to d1db043 Compare January 21, 2025 17:56
@@ -1295,7 +1296,7 @@ typedef-item ::= resource-item

func-item ::= id ':' func-type ';'

func-type ::= 'func' param-list result-list
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add some text to Wit.md briefly describing what non-blocking does, from a Wit user perspective? And maybe link back to the Explainer.md for the full details?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, fixed, PTAL

```
If a resource-type has a potentially-blocking constructor, it can simply use
`static new: func(...) -> my-resource` instead; `constructor` has no advantages
beyond more-idiomatic bindings generation in some languages.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a new Wit user perspective. the name "non-blocking" could sound like it means "no stopping the caller" rather than "no waiting for I/O", which would be confusing since it does the exact opposite of that :-}. At least we should clearly document the Wit-user-facing side of this keyword.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I suppose "blocking" does have multiple interpretations. I added this addition to the previous note, but it could probably be improved.

a `valtype`.
a `valtype`. Function types can optionally be annotated with a `non-blocking`
attribute which has no semantic effect and is ignored at validation- and
run-time, serving primarily as a hint that tells bindings generators to lift
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the "primarily" here redundant?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I suppose so. It felt a bit off without any word, so I tried "only", and also tightened up the rest of the sentence in this commit, PTAL

@badeend
Copy link
Contributor

badeend commented Jan 23, 2025

Since this is just a hint for the binding generators and not actually enforced at runtime: as a WIT author I'd rather prefer the defaults to be swapped. I.e. functions should be generated as non-blocking by default unless told otherwise.

As an example I annotated the wasi-sockets proposal with both defaults:

  • With "blocking" as the default, it needs 34 annotations
  • With non-blocking as the default, it needs just 5.

Aside from the amount of work it is for me (which I can live with :P ), annotating only the "blocking" functions is more familiar to readers who have experience with async/await languages, since the concept maps pretty much 1:1.

@dicej
Copy link
Collaborator

dicej commented Jan 23, 2025

Since this is just a hint for the binding generators and not actually enforced at runtime: as a WIT author I'd rather prefer the defaults to be swapped. I.e. functions should be generated as non-blocking by default unless told otherwise.

I said the same thing when @lukewagner and I discussed this a while ago. I believe his response was that constructors are non-blocking by convention and that, once WIT has special syntax for resource getters and setters, those will also be non-blocking by convention, which should reduce the number of explicit annotations required.

That said, I still tend to agree with you about swapping, since experience has show that functions in the average WIT interface that involve I/O are in the minority, even after you ignore the constructors, getters, and setters.

@lukewagner
Copy link
Member Author

Scanning that wasi-sockets PR, 28 of the 34 non-blocking annotations would be removed by WIT getter/setter syntax.

For the case of connect, it seems like this should be blocking (as is, e.g., the Node.js API). For the case of disconnect, I may not understand this one fully, but the docs say it's equivalent to connect() with AF_UNSPEC, and since connect() is async, it seems like it should be blocking too?

For the case of bind (x2) and listen, I see there are corresponding IORING_OP_*s for these, so if one was using an io_uring-based implementation, there could hypothetically be concurrency wins from doing these async. In general, I've been imagining that the rough (but not hard) criteria for "should it be blocking?" is "does it require a syscall?" since this where a bunch of potentially-blocking and concurrent things can happen.

But for the last case, receive, this does seem like a case we might see more of where the returned tuple<stream<u8>, future<...>> (perhaps one day soon condensed down to a stream<u8, _, error-code> or something like that) captures all the asynchrony and so the non-blocking looks superfluous. One idea is that, in the same way as "constructors are implicitly non-blocking", we could say that "functions returning only futures+streams are implicitly non-blocking" too.

@badeend
Copy link
Contributor

badeend commented Jan 25, 2025

Regarding connect/disconnect: I suspect you're looking at the UDP variants. Those do not perform any IO, so their annotations are correct. Their unfortunate names are explained here.

As for bind (2x) & listen: those do not perform any IO either. It appears their io_uring opcodes were added recently. IIUC, the reason for inclusion in io_uring isn't to do with I/O but rather for interoperability with io_uring's "registered files" feature. My knowledge of io_uring is not sufficient enough to know how this would impact the WASI interface (if at all).


I've been imagining that the rough (but not hard) criteria for "should it be blocking?" is "does it require a syscall?"

I see your point (and the "but not hard" caveat), but I don't know how to extrapolate that guidance:

  • All sockets options are syscalls too, so following that logic would imply they should all be blocking methods instead of getters/setters.
  • From the component's POV, arent't all imports effectively "syscalls"?

Having native syntax for getters & setters could indeed help alleviate many of the annotations in wasi-sockets. As you already counted out, that would bring the amount of needed annotations down from 34 to just 6. Versus 5. So: potáto, potàto. 🙃

My other point still stands though; this is a syntax is directed at binding generators / language implementors. IMO it still makes sense to speak "their" language, not "ours". From a JavaScript/Python/Rust/C#/.. developer's POV, seeing an async annotation is instantly familiar and (aside from an edge case here and there) will probably map 1:1 to async in the generated bindings for their language. That being said, I realize this is purely syntactical bikeshedding so please don't block the PR on this, as I fully agree with the semantics. 👍

@lukewagner
Copy link
Member Author

lukewagner commented Jan 27, 2025

@badeend Good points!

Revisiting the case of receive with fresh eyes (and anticipating more like it), the workaround I suggested above seems too implicit; a "blocking implies you get async" rule does seem much simpler and, to your point, "speaking their language". So despite starting from an initial position that we should async everything, I'm starting to come around to your (and Joel's) POV that maybe non-blocking is the better default and blocking should require the explicit opt-in.

Happy to hear more thoughts on this!

@lukewagner
Copy link
Member Author

Polling more folks during the BA component-model implementation meeting, lots of agreement on switching the default. But the meeting also highlighted that @sunfishcode was right above in saying that "blocking" can naturally be interpreted to have the opposite meaning that I'm assuming in the PR. Thinking of what a good short intuitive alternative keyword is, async seems like the obvious choice. Of course some languages (like Go) don't have async, but these same languages are ignoring this hint anyways (at least in generated bindings; a Go programmer may still benefit from seeing the hint to know that they might want to go call() it instead of just call()ing it).

Thus, in summary, it seems like a good idea to default to synchronous bindings with an explicit opt-in via async. I'll update the PR in a bit, and happy to hear more thoughts.

@sunfishcode
Copy link
Member

Virtualization adds an interesting perspective to this, because people may sometimes make APIs that are fine to be synchronous initially, but which other people later want to virtualize using slower implementations. A historic example of this is Unix thinking that disks are fast and making all filesystem APIs synchronous. Then when they tried to "virtualize" it with NFS it had to try to fake synchronous semantics while being async under the covers, which got awkward.

Also, "async" may give people the wrong intuition with respect to wasm's colorless async composition. Sync code can still call these "async" functions, using sync bindings.

I'm fond of the idea of making "blocking" (aka "async") the default, provided we can find a sufficiently intuitive keyword for "non-blocking". How does "shallow" sound?

bind: shallow func(local-address: ip-socket-address) -> result<_, error-code>;

"Shallow" evokes the idea that this function doesn't have a deep call tree; it just configures a local data structure and then returns. Perhaps not obvious enough to intuit from first principles, but might feel natural once learned.

@lukewagner
Copy link
Member Author

I also like the idea of "async by default", but the monkey wrench for me was cases like receive, which return a stream or future, and where it feels silly to have the generated async bindings force you to asynchronously wait to get that stream/future (which you then asynchronously wait on again). But then writing non-blocking (or whatever we want to call it) on listen looks confusing too (b/c the overall operation is blocking, even if the initial call isn't). And my workaround (of special-casing functions-returning-futures/streams) seemed too subtle and also brittle.

Also, while I also worry about async making people think we have the "what color is your function?" problem, I expect it won't be such a problem as to cause people to bounce off it entirely (before they learn that it's just a hint and thus not a problem).

@cpetig
Copy link

cpetig commented Feb 2, 2025

Given that the new opt-in keyword (blocking or async) would change behavior at the ABI level, and in this case the no-keyword case would remain ABI identical to 0.2 while async introduces more call overhead, I feel that making non-async the default results in less surprises.

I was enthusiastic about an async ABI default as well, but it clearly seems the less frequent choice once future and stream return types are available.

@sunfishcode
Copy link
Member

Given that the new opt-in keyword (blocking or async) would change behavior at the ABI level, and in this case the no-keyword case would remain ABI identical to 0.2 while async introduces more call overhead, I feel that making non-async the default results in less surprises.

Ah, that's true.

Initially I was thinking that the alternative would be to just have bindings generators default to sync bindings. That would also eliminate the same surprises. But the place where this breaks is if you have a mix of p2 and p3 APIs, and you want to use async bindings for the p3 APIs, but that would make all your p2 APIs use async bindings too, which they likely weren't designed for.

In other words, we have this table:

behavior p2 p3
blocking use a pollable etc. blocking
non-blocking default non-blocking

Which shows how if we made blocking the default in p3, it would conceptually represent a change in the default.

I was enthusiastic about an async ABI default as well, but it clearly seems the less frequent choice once future and stream return types are available.

I don't think future reduces the need for blocking though. In this code:

foo: blocking func() -> s32;
foo: non-blocking func() -> futue<s32>;

The blocking version gives both caller and callee a choice of whether to use sync or async bindings. The future version essentially requires both to use async. So between these two I think we're usually going to prefer the blocking version.

@sunfishcode
Copy link
Member

And following up on my comment here, since it makes sense for the default to be non-blocking, "shallow" is the wrong sense, and I agree that it makes sense to call the keyword "async" for familiarity.

sunfishcode referenced this pull request in WebAssembly/wasi-clocks Feb 7, 2025
@lukewagner lukewagner changed the title Add 'non-blocking' function attribute Add an 'async' hint to functions Feb 19, 2025
@lukewagner
Copy link
Member Author

Ok, I updated the PR to flip defaults and use async, PTAL.

Working on this, I realized that there is a much better place to put a hint that doesn't affect static or dynamic semantics and only informs bindgen: in the import/export name string. Thus, I updated the PR to encode the async hint as [async]foo, [async method]R.foo or [async static]R.foo. The fact that you can't have an async constructor falls out from the fact that there is no [async constructor] allowed. My impression is that this approach is also slightly easier for the toolchain, but let me know if not and happy to reconsider the "attribute on function type" approach if there are issues with doing it in the name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants