Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:126427
Precedence: bulk
MIME-Version: 1.0
References: <CAMW7n8CcNuttyxhtDeG1apHFVjU_9sKAhf0RG890b82Q1AnJpw@mail.gmail.com>
 <CAEKnhAFX-RnmRwgP_OoMs_fV2J_NWrfbEvUMDqMwgp_Td4JrOQ@mail.gmail.com>
 <CAMW7n8AC_VAg0eOZFBorma2jFX=5eS0TCSOP2sY67Dz6BiQyjQ@mail.gmail.com>
 <CAMW7n8D5kEB88qJiV8dx5fP9KGbaUa6Xc9znjqYcNOjG4oiXBQ@mail.gmail.com> <ccd08640-b873-4a22-94f3-ce8ef47bdbe8@app.fastmail.com>
In-Reply-To: <ccd08640-b873-4a22-94f3-ce8ef47bdbe8@app.fastmail.com>
Date: Mon, 17 Feb 2025 11:18:00 +0200
Message-ID: <CAMW7n8CAmPxed3kB3gCL3Bzdwho-_SQy4x-5FSjub-TBukEm6Q@mail.gmail.com>
Subject: Re: [PHP-DEV] PHP True Async
To: Rob Landers <rob@bottled.codes>
Cc: internals@lists.php.net
Content-Type: multipart/alternative; boundary="000000000000ca20b0062e53001b"
From: edmond.ht@gmail.com (Edmond Dantes)

--000000000000ca20b0062e53001b
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

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=E2=80=94an 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=E2=80=99ve thought about this. It really doesn=E2=80=99t seem like t=
he 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=E2=80=99re 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*=E2=80=94I
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 =3D match (Async\select($channel)) {
>  Async\EOF =3D> "channel closed",
 > default =3D> $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=E2=80=94the programmer must close it explicitly,=
 or the
*GC* will do it.

Thanks for the great comments!  They will help make this product better.

--000000000000ca20b0062e53001b
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div>Hello, Rob!</div><div><br></div><div class=3D"gmail_q=
uote gmail_quote_container"><blockquote class=3D"gmail_quote" style=3D"marg=
in:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1e=
x"><div class=3D"msg-630713842204985564"><div id=3D"m_-630713842204985564si=
g121229152">=C2=A0It is probably best to have channels be non-buffered by d=
efault (like in Go); buffered channels can hide architectural issues in asy=
nc code.</div></div></blockquote><div>

Great point! If a channel has a default capacity greater than 1, it leads t=
o implicit behavior that the user must be aware of. If something results in=
 implicit behavior, it&#39;s a bad practice. I&#39;ll fix it!=C2=A0</div><d=
iv><br></div><div>&gt; Why do we need &quot;finishing&quot; producing and c=
onsuming? Won&#39;t &quot;closing&quot; the channel be enough, or does clos=
ing a channel prevent consumers from reading pending items?</div><p>Because=
 additional methods allow us to express explicit semantics while also perfo=
rming extra checks. Although, in essence, it&#39;s the same as the <code>cl=
ose()</code> method.</p><div>I&#39;m not 100% sure whether this implementat=
ion is the right choice because it has more elements compared to Go. Howeve=
r, the <code>close()</code> method has an ambiguity regarding whether we wa=
nt to terminate the Consumer or the Producer. And ambiguity leads to errors=
.=C2=A0=C2=A0</div><div><br></div><div>&gt; Personally, it should be an exc=
eption (or at least a warning) if a channel gets GC&#39;d with pending data=
.</div><div><br></div><div><p>Yes, we need to think about what would be bet=
ter=E2=80=94an exception or a warning.</p><p>The GC considers whether the c=
hannel was explicitly closed or not.<br>However, if someone tries to close =
consumption while there is still data, a warning is mandatory because this =
is, once again, <strong>explicit</strong> semantics. If a programmer wants =
to close a channel with data, they must first call <code>discardData()</cod=
e> and only then close it.</p><p>So yes, explicit operations are much bette=
r.</p><p>&gt; Looking at the public api, it seems that it is very hard to u=
se 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.</p><p>=
=C2=A0 What exactly is difficult to use?=C2=A0=C2=A0</p><p>&gt; Can we name=
 this Async\Closure or something similar? Just to keep it inline with the c=
urrent \Closure</p><p>=C2=A0 It&#39;s hard for me to judge this. On one han=
d, the name <strong>Callback</strong> clearly reflects what this object doe=
s.</p><p>=C2=A0 &gt; Speaking of, you probably meant to use \Closure as the=
 $callback property instead of mixed?</p><p>=C2=A0 Unfortunately, PHP does =
not support the <strong>callable</strong> type for properties.</p><p>=C2=A0=
 &gt; - The constructor for a Future is public, but it looks like it would =
be weird to use.</p><p>I&#39;m currently working on this module and decided=
 to base it on the implementation in AMPHP. So, the <code>Future</code> cla=
ss will just be a wrapper around <code>FutureState</code>. And yes, its con=
structor can remain public, allowing <code>Future</code> to be used as <cod=
e>DeferredFuture</code>.</p><p>=C2=A0&gt; It would be nice to see this simp=
ly use Futures as much as possible. For example, returning a Future instead=
 of FiberHandles.</p><p>=C2=A0 Yes, this will be implemented a bit later, a=
fter the <strong>Future</strong> module is fully ready. Most likely, this w=
eek.</p><p>=C2=A0 &gt;=C2=A0

Is onSignal OS signals or application signals? (I didn&#39;t look at the C =
code) If the latter, it is probably better to use something other than inte=
ger. If the former, this is usually handled by extensions (such as pcntl) a=
nd comes with a ton of caveats. It might be better to let those extensions =
handle that.

</p><p>Yes, these are OS signals, but they are essentially cross-platform. =
Can <code>int</code> be replaced with something else, like an <code>enum</c=
ode>? Of course.</p><p>&gt; - Exposing $reserved -- which is accessible via=
 reflection -- is probably a bad idea.</p><p>Yes, I=E2=80=99ve thought abou=
t this. It really doesn=E2=80=99t seem like the best idea.</p><p>I might en=
d up making the <code>Notifier</code> object final, which would eliminate t=
his issue.</p><p>&gt; Should this be implementing Stringable interface? Als=
o, this will automatically cast to a string if accidentally passed to a fun=
ction that takes a string -- unless you are using strict mode.=C2=A0<a href=
=3D"https://3v4l.org/3AqcW" target=3D"_blank">https://3v4l.org/3AqcW</a>=C2=
=A0This is probably not desired.</p><p>Yes, all <code>Notifier</code> class=
es have a string representation for analysis purposes.</p><p>Do you think i=
t would be better to use a separate function instead of <code>__toString</c=
ode> to avoid implicit behavior? If it helps prevent unintended behavior, t=
hen you=E2=80=99re probably right.</p><p>&gt; Instead of Terminate(), shoul=
d it be Close(), to be consistent with the rest of the library?</p><p>=C2=
=A0 Yes, maybe <code>close()</code> is better.</p><p>&gt; - It would be nic=
e to get a list of callbacks on the Notifier as well.</p><p>

I 100% agree. I&#39;ll add this method now before I forget.=C2=A0</p><p>=C2=
=A0&gt; It would be nice to be able to get a future from the Walker instead=
 of polling &quot;isFinished&quot;.<br>Of course.</p><p>&gt;=C2=A0 IMHO, it=
 would be better to implement this around Futures (see amphp&#39;s extensiv=
e library around this). Here are some examples from my own code:</p><p>=C2=
=A0 Sorry, but for some reason, I don&#39;t see the code. But just in case,=
 I can say in advance that <code>Walker</code> was essentially made thanks =
to <strong>AMPHP</strong>=E2=80=94I borrowed part of the implementation fro=
m there :)</p><p>=C2=A0 &gt;=C2=A0

timeouts: race two futures, one which times out and the other which perform=
s 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 t=
imeout.</p><p>=C2=A0 Why do you need <code>Walker</code> for this?</p><p>=
=C2=A0 &gt;=C2=A0

- 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; sometime=
s we want to stop on the first error; and sometimes we want to just get the=
 first success (see timeouts).<br>=C2=A0 This looks more like <code>AwaitAl=
l()</code> / <code>AwaitAny()</code>. These functions will also be availabl=
e once I finish <code>Future</code>.</p><p>&gt; $result =3D match (Async\se=
lect($channel)) {</p><div>&gt;=C2=A0 Async\EOF =3D&gt; &quot;channel closed=
&quot;,<br></div><div>=C2=A0&gt; default =3D&gt; $channel-&gt;receive(),<br=
></div><div>&gt;}<br><br>Why?</div></div><div><br></div><div>foreach ($chan=
nel as $data) {<br>=C2=A0 =C2=A0 // ...<br>}<br></div><div><br></div><div>j=
ust use it :)</div><div><br></div><div>&gt; Also, keep in mind this is pret=
ty valid</div><div>=C2=A0 That&#39;s exactly how it&#39;s implemented now. =
The channel does not treat <code>NULL</code> as the end of transmission=E2=
=80=94the programmer must close it explicitly, or the <strong>GC</strong> w=
ill do it.</div><div><br></div><div>

Thanks for the great comments!=C2=A0=20

They will help make this product better.

=C2=A0=C2=A0</div></div></div>

--000000000000ca20b0062e53001b--