Hello, Rob!
It is probably best to have channels be non-buffered by default (like in Go); buffered channels can hide architectural issues in async code.
Great point! If a channel has a default capacity greater than 1, it leads to implicit behavior that the user must be aware of. If something results in implicit behavior, itâs a bad practice. Iâll fix it!
Why do we need âfinishingâ producing and consuming? Wonât âclosingâ the channel be enough, or does closing a channel prevent consumers from reading pending items?
Because additional methods allow us to express explicit semantics while also performing extra checks. Although, in essence, itâs the same as the close() method.
Iâm not 100% sure whether this implementation is the right choice because it has more elements compared to Go. However, the close() method has an ambiguity regarding whether we want to terminate the Consumer or the Producer. And ambiguity leads to errors.
Personally, it should be an exception (or at least a warning) if a channel gets GCâd with pending data.
Yes, we need to think about what would be betterâan exception or a warning.
The GC considers whether the channel was explicitly closed or not.
However, if someone tries to close consumption while there is still data, a warning is mandatory because this is, once again, explicit semantics. If a programmer wants to close a channel with data, they must first call discardData() and only then close it.
So yes, explicit operations are much better.
Looking at the public api, it seems that it is very hard to use correctly. It would probably be a good idea to pair it down and come up with an api/ideomatic usage that makes it hard to use incorrectly.
What exactly is difficult to use?
Can we name this Async\Closure or something similar? Just to keep it inline with the current \Closure
Itâs hard for me to judge this. On one hand, the name Callback clearly reflects what this object does.
Speaking of, you probably meant to use \Closure as the $callback property instead of mixed?
Unfortunately, PHP does not support the callable type for properties.
- The constructor for a Future is public, but it looks like it would be weird to use.
Iâm currently working on this module and decided to base it on the implementation in AMPHP. So, the Future class will just be a wrapper around FutureState. And yes, its constructor can remain public, allowing Future to be used as DeferredFuture.
It would be nice to see this simply use Futures as much as possible. For example, returning a Future instead of FiberHandles.
Yes, this will be implemented a bit later, after the Future module is fully ready. Most likely, this week.
Is onSignal OS signals or application signals? (I didnât look at the C code) If the latter, it is probably better to use something other than integer. If the former, this is usually handled by extensions (such as pcntl) and comes with a ton of caveats. It might be better to let those extensions handle that.
Yes, these are OS signals, but they are essentially cross-platform. Can int be replaced with something else, like an enum? Of course.
- Exposing $reserved â which is accessible via reflection â is probably a bad idea.
Yes, Iâve thought about this. It really doesnât seem like the best idea.
I might end up making the Notifier object final, which would eliminate this issue.
Should this be implementing Stringable interface? Also, this will automatically cast to a string if accidentally passed to a function that takes a string â unless you are using strict mode. https://3v4l.org/3AqcW This is probably not desired.
Yes, all Notifier classes have a string representation for analysis purposes.
Do you think it would be better to use a separate function instead of __toString to avoid implicit behavior? If it helps prevent unintended behavior, then youâre probably right.
Instead of Terminate(), should it be Close(), to be consistent with the rest of the library?
Yes, maybe close() is better.
- It would be nice to get a list of callbacks on the Notifier as well.
I 100% agree. Iâll add this method now before I forget.
It would be nice to be able to get a future from the Walker instead of polling âisFinishedâ.
Of course.
IMHO, it would be better to implement this around Futures (see amphpâs extensive library around this). Here are some examples from my own code:
Sorry, but for some reason, I donât see the code. But just in case, I can say in advance that Walker was essentially made thanks to AMPHPâI borrowed part of the implementation from there 
timeouts: race two futures, one which times out and the other which performs a long-running task. If the timeout wins, then I cancel the long-running task and proceed as an error. If the long-running task wins, I cancel the timeout.
Why do you need Walker for this?
- wait for all to complete: there are a couple of variations on this theme. But sometimes we want to wait for all to complete, errors or not; sometimes we want to stop on the first error; and sometimes we want to just get the first success (see timeouts).
This looks more like AwaitAll() / AwaitAny(). These functions will also be available once I finish Future.
$result = match (Async\select($channel)) {
Async\EOF => âchannel closedâ,
default => $channel->receive(),
}
Why?
foreach ($channel as $data) {
// âŚ
}
just use it
Also, keep in mind this is pretty valid
Thatâs exactly how itâs implemented now. The channel does not treat NULL as the end of transmissionâthe programmer must close it explicitly, or the GC will do it.
Thanks for the great comments! They will help make this product better.