Hi, On Fri, Oct 31, 2025 at 10:40 AM Tim Düsterhus <[email protected]> wrote:
> Hi > > Am 2025-10-30 22:06, schrieb Jakub Zelenka: > > 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 > > 1. > > Thank you for the RFC. I've taken a first skim of the proposal and it > immediately raised the question of naming and namespacing in particular. > Our naming policy at > https://github.com/php/policies/blob/main/coding-standards-and-naming.rst#bundled-ewill > be alwaysxtensions > <https://github.com/php/policies/blob/main/coding-standards-and-naming.rst#bundled-extensions> > says that “namespaces SHOULD be used” and given that this is a > completely new API, I think we should namespace them. > > My understanding is that the proposed API relies on a file descriptor > and not something like a timeout. It therefore makes sense to me to put > it into a `namespace Io\Poll;` or similar. We would then also have: > > namespace Io; > class IoException extends \Exception {} > namespace Io\Poll; > class PollException extends \Io\IoException {} > > I thought about this and think this might be a good idea. Just to note internally it might not always be a fd. For example, TimerHandle might implement internal API not requiring fd because some platforms (e.g. kqueue ones) don't use it but from the API design (and on Linux), it is fd based so putting that to Io namespace might make sense. The thing is that I also started working on new IO copy API: https://github.com/php/php-src/compare/master...bukka:php-src:io_copy . This is just a stub but it aims to introduce new IO layer (initial mainly for copying but potentially cover more operations) and it should also contain a new IO ring (on Linux based on io_uring) variant that could be potentially also exposed in some form. I will get it to some working form in the coming weeks and then thing how to organise it with the poll. > The StreamPollHandle method should possibly be placed in the global > namespace still, since the stream functions are sitting there - and a > SocketPollHandle would of course be sitting in the namespace of the > Sockets extension. > Yeah those could stay in global and just extend Io\Poll\Handle. > 2. As for the PollBackend enum. > > Is there a reason why this is a backed enum? Generally speaking enums > should not be backed, unless there is a good reason for this. I don't > think there is in this case - and none of the native enums are backed so > far. > > I missed that they should not be backed. I just saw `enum AdjacentPosition : string` in Dom so thought that it's fine to use it... Will change it. > 3. Exception-wise. > > StreamPollHandle::__construct(): This should probably be a ValueError, > not a PollException, since passing an invalid stream is a programmer > error. > This makes sense. > > In the other cases it probably makes sense to further split the > PollException into purpose-built exceptions according to the Throwable > policy at > > https://github.com/php/policies/blob/main/coding-standards-and-naming.rst#throwables > (“The exception message MUST NOT be the only means of distinguishing > exception causes that the user might want to handle differently.”). > > This makes sense and I will introduce more exceptions. I will not use exception per errno but some middle ground is a good idea. Maybe per op exception with codes representing specific errors would be make sense? > As an example PollContext::__construct() should probably throw a > BackendUnavailableException or something like this. For > PollContext::add() I'm wondering in which cases a handle “cannot be > added”. It fails if the same fd is added. This is also limitation of backends (e.g. epoll does not allow the same fd to be added twice). I got error code for the specific errors so this should be probably exposed as well. It would still make sense to differentiate that it's an exception for addition. > Is this an error situation that users will encounter in the real > world? Similarly, when can PollContext::wait() fail? > Wait might also fail but less likely. > > 4. PollBackend > > Is the availability of the backends known at compile time of PHP or at > runtime only? Depending on that it might make sense to only > conditionally define the enum cases, allowing users to check > availability with `defined()` or checking the output of `::cases()`. > Alternatively, a `public static function getAvailableBackends(): array` > could be added. > > It's compile time but not sure if I like exposing enum only if compiled in as it makes the checks harder. This part is not really something that users should use but it's really more for testing purpose. In reality everyone should just use default Auto... But getAvailableBackends() might make sense even for testing. Maybe it could also have per enum method isAvailable() so user can check that but not sure if it's needed. Kind regards, Jakub
