Skip to content

Fall back to epoll_wait when epoll_pwait2 is unavailable at runtime #23825

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bukka merged 1 commit into from
Sep 29, 2026
Merged

Fall back to epoll_wait when epoll_pwait2 is unavailable at runtime #23825

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions main/poll/poll_backend_epoll.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@

#include <sys/epoll.h>

#ifdef HAVE_EPOLL_PWAIT2
/* Cleared when the running kernel returns ENOSYS */
static zend_atomic_bool epoll_pwait2_available = ZEND_ATOMIC_BOOL_INITIALIZER(true);
#endif

typedef struct epoll_backend_data {
int epoll_fd;
struct epoll_event *events;
Expand Down Expand Up @@ -180,13 +185,21 @@ static int epoll_backend_wait(
backend_data->events_capacity = max_events;
}

int nfds;
int nfds = 0;
#ifdef HAVE_EPOLL_PWAIT2
nfds = epoll_pwait2(backend_data->epoll_fd, backend_data->events, max_events, timeout, NULL);
#else
int timeout_ms = php_poll_timespec_to_ms(timeout);
nfds = epoll_wait(backend_data->epoll_fd, backend_data->events, max_events, timeout_ms);
if (EXPECTED(zend_atomic_bool_load_ex(&epoll_pwait2_available))) {
nfds = epoll_pwait2(
backend_data->epoll_fd, backend_data->events, max_events, timeout, NULL);
if (UNEXPECTED(nfds < 0 && (errno == ENOSYS || errno == ENOTSUP))) {
zend_atomic_bool_store_ex(&epoll_pwait2_available, false);
}
}
if (UNEXPECTED(!zend_atomic_bool_load_ex(&epoll_pwait2_available)))

@TimWolla TimWolla Sep 29, 2026

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.

This is not pretty: When pwait2 is unavailable at runtime, folks are paying for true branches, with one of them UNEXPECTED(). I would suggest making it a real else and then goto from line 194 into the else part, similarly to:

php-src/ext/random/zend_utils.c

Line 40 in 410e61d

goto fallback;

@bukka bukka Sep 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

but this is for HAVE_EPOLL_PWAIT2 which means a case when it was compiled with such support but it runs on older kernel later. I was kind of assuming that such case should be UNEXPECTED...

@TimWolla TimWolla Sep 29, 2026

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.

I was kind of assuming that such case should be UNEXPECTED...

My understanding is that UNEXPECTED is a very strong hint that the branch will not be taken. But that is not true: Once it is taken once, it will always be taken.

But I'm now realizing: This will likely always perform two atomic loads, even when pwait2 is actually available, since the compiler can't assume that the atomic doesn't change between the two conditions.

An explicit else (+ goto) has clearer semantics and should be faster in all cases. I wouldn't add any expected/unexpected hints, this is something that should be easily handled by the branch predictor.

@ndossche ndossche Sep 29, 2026

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.

My understanding is that UNEXPECTED is a very strong hint that the branch will not be taken. But that is not true: Once it is taken once, it will always be taken.

This is correct. Generally, UNEXPECTED tends to move the code to the cold text section, use it generally only for error-handling code.

But I'm now realizing: This will likely always perform two atomic loads, even when pwait2 is actually available, since the compiler can't assume that the atomic doesn't change between the two conditions.

Also true

@bukka bukka Sep 30, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ok, see #24026

#endif
{
int timeout_ms = php_poll_timespec_to_ms(timeout);
nfds = epoll_wait(backend_data->epoll_fd, backend_data->events, max_events, timeout_ms);
}

if (nfds > 0) {
Comment thread
bukka marked this conversation as resolved.
for (int i = 0; i < nfds; i++) {
Expand Down
Loading