Feedback from Billy (split into multiple issues)

[ericniebler]

Issue by brycelelbach
Monday Sep 27, 2021 at 21:45 GMT
Originally opened as NVIDIA/stdexec#195

---

This should be split into multiple issues:

1. http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2300r1.html#example-async-inclusive-scan If you want this to be a valid implementation of inclusive-scan we need to fix http://eel.is/c++draft/algorithms.parallel.exceptions to stop special casing whether the user throws bad alloc or we do; under the current rules it would require an implementation to make a duplicate copy of all the execution machinery to propagate a different exception type to distinguish between the algorithm's failure to allocate and user std::bad_allocs. (For example we do that by replacing vector's allocator in our implementation today https://github.com/microsoft/STL/blob/dc888f7d9fb7a4db8d3441f9e9bac2e0c6ecc4db/stl/inc/execution#L282-L313 ) . Considering [algorithms.parallel.exceptions] was supposedly written to make things easier for implementers, I would prefer to not change the example there, and instead fix [algorithms.parallel.exceptions] to say that execptions occurring on calling thread(s) may be propagated to the calling thread(s) and exceptions emitted on threads created by the implementation go to terminate go directly to terminate ...
2. I also observe that that implementation does the terrible "barrier" inclusive_scan algorithm; is something like https://research.nvidia.com/sites/default/files/pubs/2016-03_Single-pass-Parallel-Prefix/nvr-2016-002.pdf implementable in this universe? (MSVC's, and presumably Thrust's inclusive_scan use that algorithm)
3. http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2300r1.html#example-async-dynamically-sized-read It would be neat if what happens for errors was described here. While that may distract from the async concepts trying to be described, how to bail out correctly on failure is an important consideration for async things. (And indeed, cancellation being difficult is one of the primary criticisms against the ASIO design proposed for standardization right now) Just replacing the asserts with what a real implementation would do instead may be sufficient?
4. http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2300r1.html#design-transitions RE: "running user code anywhere but where they defined it to run must be considered a bug" there are always iterator copies etc. that likely happen when setting up the parallel algorithm calls. Be very careful about specifying what "user code" means here. The thing that made [algorithms.parallel.exceptions] so frustrating to implement is that things people don't expect to throw, like copying an iterator, can nonetheless throw.
5. http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2300r1.html#design-fpg I am so happy to see forward progress guarantees being discussed 😄
6. RE: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2300r1.html#design-sender-adaptor-ensure_started & co.: I'm concerned that this creates new avenues of creating "detached" work, which seems like an "always [basic.start.term]/6 timebomb". I note that the compromise that added thread::detach() also added Xxx_at_thread_exit as a hypothetical way to restore join-like semantics to detached work. but there appears to be no such help here. Of note, transferring work off an execution agent does not mean that the execution agent is actually dead, but [basic.start.term]/6 (and real implementations) require that the execution agent is actually dead.
7. http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2300r1.html#design-dispatch I suspect I am just drowned out here but I really hate the escalating war that we have declared on ADL and the problems we've created for ourselves resulting like this. I note that the reason tag_invoke died in LEWGi in San Diego was due to adding an additional layer of std::forward calls needed for everything; it is unfortunate in my view that that concern is unaddressed.
8. Quasi-related to this paper, we talk about "execution context" in lots of places and I see SG1 people seem to know exactly what that means; I still do not and suspect I am not alone. I note that what this paper thinks that is and what the networking TS thinks that is are not the same.
9. http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2300r1.html#spec-execution.op_state.start Are there any requirements on what happens for connect relating to the started work? I'm asking because we would want to use this to implement the "prepare for parallelism / commit to parallelism / on failure fall back to serial" behavior, but if connect() can start the work we're toast. Or is that always going to be under control of the standard library for purposes of the existing parallel algorithms? Example https://github.com/microsoft/STL/blob/dc888f7d9fb7a4db8d3441f9e9bac2e0c6ecc4db/stl/inc/execution#L1189-L1233 -- everything up to line 1206 is partitioning the work etc. which may require allocation memory or threads or similar, and if any of that fails we throw _Parallelism_resources_exhausted, and fall down to serial. Of note, that design only works if none of the for_each invocations have run yet
10. http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2300r1.html#spec-execution.senders.adaptors.bulk It would be good to note here that "shape" is "number of partitions". Perhaps that should be changed to be partition_count and it can be renamed back to shape should the hypothetical non-integral shapes present themselves.
11. http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2300r1.html#spec-execution.senders.consumers.sync_wait needs to "block with forward progress delegation" in several circumstances to implement the existing parallel algorithms library if we want it to.

[ericniebler]

Comment by griwes
Monday Sep 27, 2021 at 23:06 GMT

---

Re: 6, I don't know how to answer that, not really.

To speak to start_detached:

We could remove it, but I believe that that is deeply problematic. There's valid programs that want to just fire work off, ones that do not worry about clean shutdown and its friends. It's possible that at the end of a sender chain you call into a C API that will then trigger additional work in a manner that is either inconvenient or very complex to adapt into senders proper, so there may be nothing to re-join with.

To speak to ensure_started:

I do believe that this is far less problematic, at least regarding the issues Billy raised: it still returns you a direct handle to the completion of the work you've started. It just forces a thing to be put into a queue. While I could potentially be persuaded that start_detached shouldn't be a thing (though that would need to be an extremely good argument, that directly addresses the next paragraph), I don't think we could ever get rid of ensure_started - not unless we are actually okay with leaving performance (or at the very least, latency) on the floor (and it's possible that we are okay with it - I just don't think that's the case).

If we remove it, I sincerely believe that real programs will contain user-provided implementations of the same functionality, and that's actually really bad, because it means that schedulers can no longer customize what happens when someone invokes start_detached or ensure_started, and IMHO that's a really bad place to be.

We do not need to encourage people to use these functions; hell, we can do our best to actively discourage people from using them. But that doesn't change the fact that they are useful in a certain class of programs, and not providing standard vocabulary for them would, I believe, be a grave mistake.

[ericniebler]

Comment by griwes
Monday Sep 27, 2021 at 23:11 GMT

---

Re: 7: I hear that. I'd really like to have a forwarding operator instead of std::forward. I believe that we are in a place where high-quality implementations never invoke std::forward directly in their forwarding layers (and instead #define __fwd(x) static_cast<decltype(x)>(x)). I really wish that Evolution would listen to Library explaining why forward is a problem, and that Evolution should maybe consider a forwarding operator - but the last few times this was raised, there was a nigh zero attendance from LEWG in the room, so Evolution just shrugged and said this is not a problem worth solving.

tag_invoke turns out to be a fantastic tool for wholesale forwarding all or near-all customizations for another object, and a good chunk of the requirements on receiver queries depends on this. I have been a strong proponent of properties for this same reason: they gave us a simple way to forward queries and requests. With tag_invoke, we have those same benefits, but for all CPOs in the library.

[ericniebler]

Comment by griwes
Monday Sep 27, 2021 at 23:13 GMT

---

Re: 9, the fix for #192 should include some strongly worded sentences that meant that connect itself may never start work.

[ericniebler]

Comment by griwes
Monday Sep 27, 2021 at 23:17 GMT

---

Re: 11, the intent of p5 in that section was to achieve this, but rereading all that again now, I don't think it does. We should fix that, but I'm not quite sure how to word this requirement in the face of a CPO.