Hi, On Sat, Nov 1, 2025 at 8:23 PM Larry Garfield <[email protected]> wrote:
> On Thu, Oct 30, 2025, at 4:06 PM, Jakub Zelenka wrote: > > Hello, > > > > I would like to introduce a new polling API RFC that is part of my > > stream evolution work: > > > > https://wiki.php.net/rfc/poll_api > > > > Kind regards, > > > > Jakub > > I freely admit to not being fully versed in this area, so take my feedback > with however much sodium chloride you feel is appropriate. > > - Given that a lot of people reading this are probably no more versed in > kernel IO polling than I am, a section early on explaining the context of > what is even being discussed would be most appreciated. > > I added a section with some basic explanation so users that are more interested in details can easily search for it. > - I really would rather not add more global constants. Better to at > minimum make them class constants of a class that the new API provides. > (Maybe PollHandle?) > > I replaced them with \Io\Poll\Event enum so this should be covered. There are no global constants. The only constants are now on class and they are for error codes. > - Conversely, I'm unclear why the PollBackend is an enum. That implies > the list of backend implementations is fixed and immutable, and not > extensible now or in the future. I find that claim suspect, as there are > six already. (I find that following the 0-1-many rule in most cases pays > off in the long run.) > This is unlikely to grow and if it ever grew, it would happen only as a new feature in new minor version. Or do we have any policy that disallows extending enums? This is just a theoretical concern though because all the main backends are covered. The only one that could be potentially added is a different variant for Windows if WSAPoll shows poor performance as it was the case in past but not sure if anyone will do such work. > - PollWatcher is created only by PollContext. OK, then please list > PollContext first so that reading PollWatcher I have the, er, context for > where it fits. > > Updated. > - Speaking of, when there's very long code blocks like this I much prefer > to break it up to a block per class, so as to minimize codeblock > scrolling. That makes it much easier to read and jump around as I figure > out how it all works. > > Fixed. > - How would requesting a specific poll backend be helpful, if it varies by > OS? If I'm running on Windows, asking for the Linux backend wouldn't help > me much, or vice versa. I don't see the use case here. (Ie, please > describe the use case in more detail.) > There is poll supported on all unix variants and then Illumos has also variant of epoll so there are more than one backend for each platform (except Windows that currently support just one). As noted in the RFC, setting of the backend is mostly useful for testing. The actual enum is more useful to get info of the selected backend. Currently the most useful thing is to find out whether edge triggering is possible. > - Who is the target audience for this? I'm pretty sure it's not anything > I normally work on, so it's hard for me to judge if certain decisions are > good, bad, or "sucky but we have to." Eg, getData() returning "who the > hell knows" strikes me as a footgun in waiting, but I don't have enough > context to know if that's an inherited problem from elsewhere. > Added a section explaining the motivation and possible users of this API. In terms of getData, we would need to have generics to make it typed as this is for user data. You can see the examples that should give you hopefully some idea how this can be used. Kind regards, Jakub
