[PATCH 0/2] MR7392: server: Add an epoll_pwait2 path for server_main_loop if supported.

[William Horvath (＠whrvt)]

This mitigates a side effect of the global current_time and monotonic_time being updated on every server call's timeout, where the end time of any unrelated server call is moved into the future (depending on the frequency of server calls).

By using a more granular timeout, the overall frequency of server calls doesn't have as great of an effect on each individual timeout, as we don't have to wait for an entire millisecond (which was due to the ceiling operation in get_next_timeout).

This [issue](https://bugs.winehq.org/show_bug.cgi?id=57849) gives a few examples of this causing significant degradation in the behavior of some games, which use alertable (server) waits for their FPS limiter.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7392

[PATCH 1/2] server: Convert to and use a timespec directly for timeouts if supported.

[William Horvath]

From: William Horvath <william(a)horvath.blog>

Instead of first converting to milliseconds, then back to a nanosecond-precision
timespec.

This mitigates a side effect of the global current_time and monotonic_time
being updated on every server call's timeout, where the end time of any unrelated
server call is moved into the future (depending on the frequency of server calls).

By using a more granular timeout, the overall frequency of server calls doesn't have
as great of an effect on each individual timeout, as we don't have to wait for an
entire millisecond (which was due to the ceiling operation in get_next_timeout).

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57849
---
 server/fd.c | 76 +++++++++++++++++++++++++++++------------------------
 1 file changed, 42 insertions(+), 34 deletions(-)

diff --git a/server/fd.c b/server/fd.c
index ce32e7f8397..46217dbdd77 100644
--- a/server/fd.c
+++ b/server/fd.c
@@ -662,27 +662,22 @@ static inline void remove_epoll_user( struct fd *fd, int user )
 
 static inline void main_loop_epoll(void)
 {
-    int i, ret, timeout;
+    int i, ret;
+    BOOL infinite;
+    struct timespec ts;
     struct kevent events[128];
 
     if (kqueue_fd == -1) return;
 
     while (active_users)
     {
-        timeout = get_next_timeout();
+        infinite = !get_next_timeout_ts( &ts );
 
         if (!active_users) break;  /* last user removed by a timeout */
         if (kqueue_fd == -1) break;  /* an error occurred with kqueue */
 
-        if (timeout != -1)
-        {
-            struct timespec ts;
-
-            ts.tv_sec = timeout / 1000;
-            ts.tv_nsec = (timeout % 1000) * 1000000;
-            ret = kevent( kqueue_fd, NULL, 0, events, ARRAY_SIZE( events ), &ts );
-        }
-        else ret = kevent( kqueue_fd, NULL, 0, events, ARRAY_SIZE( events ), NULL );
+        if (infinite) ret = kevent( kqueue_fd, NULL, 0, events, ARRAY_SIZE( events ), NULL );
+        else ret = kevent( kqueue_fd, NULL, 0, events, ARRAY_SIZE( events ), &ts );
 
         set_current_time();
 
@@ -764,28 +759,24 @@ static inline void remove_epoll_user( struct fd *fd, int user )
 
 static inline void main_loop_epoll(void)
 {
-    int i, nget, ret, timeout;
+    BOOL infinite;
+    int i, nget, ret;
+    struct timespec ts;
     port_event_t events[128];
 
     if (port_fd == -1) return;
 
     while (active_users)
     {
-        timeout = get_next_timeout();
+        infinite = !get_next_timeout_ts( &ts );
+
         nget = 1;
 
         if (!active_users) break;  /* last user removed by a timeout */
         if (port_fd == -1) break;  /* an error occurred with event completion */
 
-        if (timeout != -1)
-        {
-            struct timespec ts;
-
-            ts.tv_sec = timeout / 1000;
-            ts.tv_nsec = (timeout % 1000) * 1000000;
-            ret = port_getn( port_fd, events, ARRAY_SIZE( events ), &nget, &ts );
-        }
-        else ret = port_getn( port_fd, events, ARRAY_SIZE( events ), &nget, NULL );
+        if (infinite) ret = port_getn( port_fd, events, ARRAY_SIZE( events ), &nget, NULL );
+        else ret = port_getn( port_fd, events, ARRAY_SIZE( events ), &nget, &ts );
 
 	if (ret == -1) break;  /* an error occurred with event completion */
 
@@ -876,10 +867,11 @@ static void remove_poll_user( struct fd *fd, int user )
     active_users--;
 }
 
-/* process pending timeouts and return the time until the next timeout, in milliseconds */
-static int get_next_timeout(void)
+/* process pending timeouts and return the time until the next timeout with nanosecond precision
+ * returns FALSE to signal infinite timeouts, TRUE otherwise */
+static BOOL get_next_timeout_ts( struct timespec *ts )
 {
-    int ret = user_shared_data ? user_shared_data_timeout : -1;
+    timeout_t next_timeout = user_shared_data ? user_shared_data_timeout : -1;
 
     if (!list_empty( &abs_timeout_list ) || !list_empty( &rel_timeout_list ))
     {
@@ -924,22 +916,38 @@ static int get_next_timeout(void)
         if ((ptr = list_head( &abs_timeout_list )) != NULL)
         {
             struct timeout_user *timeout = LIST_ENTRY( ptr, struct timeout_user, entry );
-            timeout_t diff = (timeout->when - current_time + 9999) / 10000;
-            if (diff > INT_MAX) diff = INT_MAX;
-            else if (diff < 0) diff = 0;
-            if (ret == -1 || diff < ret) ret = diff;
+            timeout_t diff = timeout->when - current_time;
+            if (diff < 0) diff = 0;
+            if (next_timeout == -1 || diff < next_timeout) next_timeout = diff;
         }
 
         if ((ptr = list_head( &rel_timeout_list )) != NULL)
         {
             struct timeout_user *timeout = LIST_ENTRY( ptr, struct timeout_user, entry );
-            timeout_t diff = (-timeout->when - monotonic_time + 9999) / 10000;
-            if (diff > INT_MAX) diff = INT_MAX;
-            else if (diff < 0) diff = 0;
-            if (ret == -1 || diff < ret) ret = diff;
+            timeout_t diff = -timeout->when - monotonic_time;
+            if (diff < 0) diff = 0;
+            if (next_timeout == -1 || diff < next_timeout) next_timeout = diff;
         }
     }
-    return ret;
+
+    /* infinite */
+    if (next_timeout == -1) return FALSE;
+
+    ts->tv_sec = next_timeout / 10000000;
+    ts->tv_nsec = (next_timeout % 10000000) * 100;
+    return TRUE;
+}
+
+/* process pending timeouts and return the time until the next timeout, in milliseconds */
+static int get_next_timeout(void)
+{
+    struct timespec ts;
+
+    if (!get_next_timeout_ts( &ts )) return -1;
+    if (ts.tv_sec >= INT_MAX / 1000) return INT_MAX;
+
+    /* ceil to avoid spinning */
+    return ts.tv_sec * 1000 + (ts.tv_nsec + 999999) / 1000000;
 }
 
 /* server main poll() loop */
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/7392

[PATCH 2/2] server: Use epoll_pwait2 if supported.

[William Horvath]

From: William Horvath <william(a)horvath.blog>

This allows higher precision waits on Linux, to match the other platforms
where we can already use a timespec directly.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57849
---
 configure.ac |  1 +
 server/fd.c  | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/configure.ac b/configure.ac
index dd139950e10..d483096fae6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2061,6 +2061,7 @@ AC_CHECK_FUNCS(\
         dladdr1 \
 	dlinfo \
 	epoll_create \
+	epoll_pwait2 \
 	fstatfs \
 	futimens \
 	futimes \
diff --git a/server/fd.c b/server/fd.c
index 46217dbdd77..2fb135d997c 100644
--- a/server/fd.c
+++ b/server/fd.c
@@ -102,6 +102,9 @@
 #if defined(HAVE_SYS_EPOLL_H) && defined(HAVE_EPOLL_CREATE)
 # include <sys/epoll.h>
 # define USE_EPOLL
+# ifdef HAVE_EPOLL_PWAIT2
+#  define USE_EPOLL_PWAIT2
+# endif
 #endif /* HAVE_SYS_EPOLL_H && HAVE_EPOLL_CREATE */
 
 #if defined(HAVE_PORT_H) && defined(HAVE_PORT_CREATE)
@@ -496,6 +499,7 @@ static int active_users;                    /* current number of active users */
 static int allocated_users;                 /* count of allocated entries in the array */
 static struct fd **freelist;                /* list of free entries in the array */
 
+static BOOL get_next_timeout_ts( struct timespec *ts );
 static int get_next_timeout(void);
 
 static inline void fd_poll_event( struct fd *fd, int event )
@@ -561,6 +565,52 @@ static inline void remove_epoll_user( struct fd *fd, int user )
     }
 }
 
+#ifdef USE_EPOLL_PWAIT2
+
+static inline void main_loop_epoll(void)
+{
+    int i, ret;
+    BOOL infinite;
+    struct timespec ts;
+    struct epoll_event events[128];
+
+    assert( POLLIN == EPOLLIN );
+    assert( POLLOUT == EPOLLOUT );
+    assert( POLLERR == EPOLLERR );
+    assert( POLLHUP == EPOLLHUP );
+
+    if (epoll_fd == -1) return;
+
+    while (active_users)
+    {
+        infinite = !get_next_timeout_ts( &ts );
+
+        if (!active_users) break;  /* last user removed by a timeout */
+        if (epoll_fd == -1) break;  /* an error occurred with epoll */
+
+        if (infinite) ret = epoll_pwait2( epoll_fd, events, ARRAY_SIZE( events ), NULL, NULL );
+        else ret = epoll_pwait2( epoll_fd, events, ARRAY_SIZE( events ), &ts, NULL );
+
+        set_current_time();
+
+        /* put the events into the pollfd array first, like poll does */
+        for (i = 0; i < ret; i++)
+        {
+            int user = events[i].data.u32;
+            pollfd[user].revents = events[i].events;
+        }
+
+        /* read events from the pollfd array, as set_fd_events may modify them */
+        for (i = 0; i < ret; i++)
+        {
+            int user = events[i].data.u32;
+            if (pollfd[user].revents) fd_poll_event( poll_users[user], pollfd[user].revents );
+        }
+    }
+}
+
+#else
+
 static inline void main_loop_epoll(void)
 {
     int i, ret, timeout;
@@ -599,6 +649,8 @@ static inline void main_loop_epoll(void)
     }
 }
 
+#endif /* USE_EPOLL_PWAIT2 */
+
 #elif defined(HAVE_KQUEUE)
 
 static int kqueue_fd = -1;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/7392

[Aida Jonikienė]

I think you forgot a `autoreconf` though

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7392#note_95418

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about server/fd.c:
> static inline void main_loop_epoll(void)
>  {
> -    int i, nget, ret, timeout;
> +    BOOL infinite;
> +    int i, nget, ret;
> +    struct timespec ts;
>      port_event_t events[128];
>
> if (port_fd == -1) return;
>
> while (active_users)
>      {
> -        timeout = get_next_timeout();
> +        infinite = !get_next_timeout_ts( &ts );

You could even return a `timespec_t *`, NULL or to the provided ts parameter, and save the `if (infinite)` below.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7392#note_95420