From ab224fb7b94bcb33596755c680c82087fd844f5c Mon Sep 17 00:00:00 2001 From: Jelle Foks Date: Mon, 14 Oct 2024 17:52:41 -0700 Subject: [PATCH 1/3] Remove obsolete Starboardization. This removes an obsolete Starboardization that introduced evtimeval that can cause an overflow on 32-bit platforms with a 64-bit time_t. --- third_party/libevent/event.c | 2 +- third_party/libevent/event.h | 18 ------------------ 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/third_party/libevent/event.c b/third_party/libevent/event.c index b151ebca969a..22bbef67e03a 100644 --- a/third_party/libevent/event.c +++ b/third_party/libevent/event.c @@ -918,7 +918,7 @@ timeout_correct(struct event_base *base, struct timeval *tv) pev = base->timeheap.p; size = base->timeheap.n; for (; size-- > 0; ++pev) { - struct evtimeval *ev_tv = &(**pev).ev_timeout; + struct timeval *ev_tv = &(**pev).ev_timeout; evutil_timersub(ev_tv, &off, ev_tv); } /* Now remember what the new time turned out to be. */ diff --git a/third_party/libevent/event.h b/third_party/libevent/event.h index b4af24e7a43a..caff09ec0538 100644 --- a/third_party/libevent/event.h +++ b/third_party/libevent/event.h @@ -204,20 +204,6 @@ typedef unsigned short u_short; #define EV_SIGNAL 0x08 #define EV_PERSIST 0x10 /* Persistant event */ -#ifdef STARBOARD -struct evtimeval { -#if SB_IS(64_BIT) - int64_t tv_sec; /* seconds */ - int64_t tv_usec; /* and microseconds */ -#else - int32_t tv_sec; /* seconds */ - int32_t tv_usec; /* and microseconds */ -#endif -}; -#else -typedef struct timeval evtimeval; -#endif - /* Fix so that ppl dont have to run with */ #ifndef TAILQ_ENTRY #define _EVENT_DEFINED_TQENTRY @@ -243,11 +229,7 @@ struct event { short ev_ncalls; short *ev_pncalls; /* Allows deletes in callback */ -#if defined(STARBOARD) - struct evtimeval ev_timeout; -#else struct timeval ev_timeout; -#endif int ev_pri; /* smaller numbers are higher priority */ From 95e0b1cc43d2883ac49a13a304de5955b6a81cd8 Mon Sep 17 00:00:00 2001 From: Jelle Foks Date: Mon, 14 Oct 2024 17:54:09 -0700 Subject: [PATCH 2/3] Remove unused member from SbSocketWaiterPrivate Remove the |waiting_| from SbSocketWaiterPrivate that was only read DCHECK() but modified twice for each wait. --- starboard/shared/libevent/socket_waiter_internal.cc | 8 +------- starboard/shared/libevent/socket_waiter_internal.h | 3 --- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/starboard/shared/libevent/socket_waiter_internal.cc b/starboard/shared/libevent/socket_waiter_internal.cc index 056cfd9a114f..261b6f5ab96d 100644 --- a/starboard/shared/libevent/socket_waiter_internal.cc +++ b/starboard/shared/libevent/socket_waiter_internal.cc @@ -122,10 +122,7 @@ void GetSocketPipe(SbSocket* client_socket, SbSocket* server_socket) { } // namespace SbSocketWaiterPrivate::SbSocketWaiterPrivate() - : thread_(pthread_self()), - base_(event_base_new()), - waiting_(false), - woken_up_(false) { + : thread_(pthread_self()), base_(event_base_new()), woken_up_(false) { #if USE_POSIX_PIPE int fds[2]; int result = pipe(fds); @@ -287,9 +284,7 @@ SbSocketWaiterResult SbSocketWaiterPrivate::WaitTimed(int64_t duration_usec) { timeout_add(&event, &tv); } - waiting_ = true; event_base_loop(base_, 0); - waiting_ = false; SbSocketWaiterResult result = woken_up_ ? kSbSocketWaiterResultWokenUp : kSbSocketWaiterResultTimedOut; @@ -362,7 +357,6 @@ void SbSocketWaiterPrivate::HandleSignal(Waitee* waitee, } void SbSocketWaiterPrivate::HandleWakeUpRead() { - SB_DCHECK(waiting_); // Remove and discard the wakeup byte. char buf; int bytes_read = HANDLE_EINTR(read(wakeup_read_fd_, &buf, 1)); diff --git a/starboard/shared/libevent/socket_waiter_internal.h b/starboard/shared/libevent/socket_waiter_internal.h index 5da590902564..f5c8c49deb95 100644 --- a/starboard/shared/libevent/socket_waiter_internal.h +++ b/starboard/shared/libevent/socket_waiter_internal.h @@ -136,9 +136,6 @@ struct SbSocketWaiterPrivate { // The registry of currently registered Waitees. WaiteesMap waitees_; - // Whether or not the waiter is actually waiting. - bool waiting_; - // Whether or not the waiter was woken up. bool woken_up_; From 8fb9f957d24777c35e55421f0b550ad52eb899f8 Mon Sep 17 00:00:00 2001 From: Jelle Foks Date: Mon, 14 Oct 2024 17:58:30 -0700 Subject: [PATCH 3/3] Avoid unnecessary timers. Avoid unnecessarily adding libevent timers. This reduces CPU utilization during a download bandwidth test by ~11%. --- .../shared/libevent/socket_waiter_internal.cc | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/starboard/shared/libevent/socket_waiter_internal.cc b/starboard/shared/libevent/socket_waiter_internal.cc index 261b6f5ab96d..1a815513ccdc 100644 --- a/starboard/shared/libevent/socket_waiter_internal.cc +++ b/starboard/shared/libevent/socket_waiter_internal.cc @@ -271,18 +271,28 @@ void SbSocketWaiterPrivate::Wait() { SbSocketWaiterResult SbSocketWaiterPrivate::WaitTimed(int64_t duration_usec) { SB_DCHECK(pthread_equal(pthread_self(), thread_)); + if ((duration_usec == 0) || (duration_usec == kSbInt64Max)) { + // There is no need for a timeout event in these cases. + event_base_loop(base_, + (duration_usec == 0) ? EVLOOP_ONCE | EVLOOP_NONBLOCK : 0); + + SbSocketWaiterResult result = woken_up_ ? kSbSocketWaiterResultWokenUp + : kSbSocketWaiterResultTimedOut; + woken_up_ = false; + + return result; + } + // The way to do this is apparently to create a timeout event, call WakeUp // inside that callback, and then just do a normal wait. struct event event; timeout_set(&event, &SbSocketWaiterPrivate::LibeventTimeoutCallback, this); event_base_set(base_, &event); - if (duration_usec < kSbInt64Max) { - struct timeval tv; - tv.tv_sec = duration_usec / 1'000'000; - tv.tv_usec = duration_usec % 1'000'000; - timeout_add(&event, &tv); - } + struct timeval tv; + tv.tv_sec = duration_usec / 1'000'000; + tv.tv_usec = duration_usec % 1'000'000; + timeout_add(&event, &tv); event_base_loop(base_, 0); @@ -290,12 +300,9 @@ SbSocketWaiterResult SbSocketWaiterPrivate::WaitTimed(int64_t duration_usec) { woken_up_ ? kSbSocketWaiterResultWokenUp : kSbSocketWaiterResultTimedOut; woken_up_ = false; - if (duration_usec < kSbInt64Max) { - // We clean this up, in case we were awakened early, to prevent a spurious - // wake-up later. - timeout_del(&event); - } - + // We clean this up, in case we were awakened early, to prevent a spurious + // wake-up later. + timeout_del(&event); return result; }