Skip to content

Use php_pollfd_for_ms more consistently for single fd polling#22872

Merged
NattyNarwhal merged 5 commits into
php:masterfrom
NattyNarwhal:poll-cleanup
Jul 24, 2026
Merged

Use php_pollfd_for_ms more consistently for single fd polling#22872
NattyNarwhal merged 5 commits into
php:masterfrom
NattyNarwhal:poll-cleanup

Conversation

@NattyNarwhal

Copy link
Copy Markdown
Member

There's a few cases where we call poll with a single fd; we have a wrapper (php_pollfd_for_ms) to make this case cleaner with less boilerplate.

Side note: said wrapper also uses the php_poll2 API, which can also emulate poll(2) using select(2); FastCGI was doing this on its own, so that also handles that case. That said, poll seems to be basically everywhere nowadays, so emulating poll is only seems useful for Windows, which has WSAPoll. I suspect php_poll2 should be a WSAPoll wrapper instead? The new poll API handles all this of course, but seems pretty heavyweight for the case of just quickly polling a single fd.

@NattyNarwhal

Copy link
Copy Markdown
Member Author

There's a few cases where we call poll with a single fd; we have a wrapper (php_pollfd_for_ms) to make this case cleaner with less boilerplate.

Side note: said wrapper also uses the php_poll2 API, which can also emulate poll(2) using select(2); FastCGI was doing this on its own, so that also handles that case. That said, poll seems to be basically everywhere nowadays, so emulating poll is only seems useful for Windows, which has WSAPoll. I suspect php_poll2 should be a WSAPoll wrapper instead? The new poll API handles all this of course, but seems pretty heavyweight for the case of just quickly polling a single fd.

FWIW looked into this further and there are only a few places where select(2) is called that outside of fallbacks if poll(2) isn't present (vendored libmagic, mysqlnd for mysqli_poll, streams_select, socket_select, dev CLI web server, CLI for polling a single fd, FPM backend), and we have a big wrapper in win32/ for reimplementing select on Windows. Not sure how useful it'd be to make those all consistent with poll or using the polling API and get rid of the reimplementations.

@arnaud-lb arnaud-lb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me otherwise

Comment thread sapi/cli/php_cli.c Outdated
This is not a poll call, but it is functionally the same as one; convert
it to the single fd polling function which is clearer to read.
@NattyNarwhal
NattyNarwhal merged commit de5a582 into php:master Jul 24, 2026
19 checks passed
@NattyNarwhal

Copy link
Copy Markdown
Member Author

Thanks, I'll take a further look at the poll/select stuff I mentioned in this thread soon enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants