Skip to content

Commit ec21b4b

Browse files
sigurasgChromium LUCI CQ
authored and
Chromium LUCI CQ
committed
Use a timer instead of a sleep loop.
This paves the way for implementing timer slack on the kqueue message pump. Bug: 1181297 Change-Id: I74fdb63e85cf726000ee94b5851f84b06de314ae Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2713848 Auto-Submit: Sigurður Ásgeirsson <siggi@chromium.org> Reviewed-by: Mark Mentovai <mark@chromium.org> Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org> Cr-Commit-Position: refs/heads/master@{#857894}
1 parent 334c7d7 commit ec21b4b

File tree

2 files changed

+64
-19
lines changed

2 files changed

+64
-19
lines changed

base/message_loop/message_pump_kqueue.cc

+56-19
Original file line numberDiff line numberDiff line change
@@ -364,25 +364,13 @@ bool MessagePumpKqueue::DoInternalWork(Delegate::NextWorkInfo* next_work_info) {
364364

365365
bool poll = next_work_info == nullptr;
366366
int flags = poll ? KEVENT_FLAG_IMMEDIATE : 0;
367-
bool indefinite =
368-
next_work_info != nullptr && next_work_info->delayed_run_time.is_max();
369-
370-
int rv = 0;
371-
do {
372-
timespec timeout{};
373-
if (!indefinite && !poll) {
374-
if (rv != 0) {
375-
// The wait was interrupted and made |next_work_info|'s view of
376-
// TimeTicks::Now() stale. Refresh it before doing another wait.
377-
next_work_info->recent_now = TimeTicks::Now();
378-
}
379-
timeout = next_work_info->remaining_delay().ToTimeSpec();
380-
}
381-
// This does not use HANDLE_EINTR, since retrying the syscall requires
382-
// adjusting the timeout to account for time already waited.
383-
rv = kevent64(kqueue_.get(), nullptr, 0, events_.data(), events_.size(),
384-
flags, indefinite ? nullptr : &timeout);
385-
} while (rv < 0 && errno == EINTR);
367+
if (!poll && scheduled_wakeup_time_ != next_work_info->delayed_run_time) {
368+
UpdateWakeupTimer(next_work_info->delayed_run_time);
369+
DCHECK_EQ(scheduled_wakeup_time_, next_work_info->delayed_run_time);
370+
}
371+
372+
int rv = HANDLE_EINTR(kevent64(kqueue_.get(), nullptr, 0, events_.data(),
373+
events_.size(), flags, nullptr));
386374

387375
PCHECK(rv >= 0) << "kevent64";
388376
return ProcessEvents(rv);
@@ -445,6 +433,12 @@ bool MessagePumpKqueue::ProcessEvents(int count) {
445433
if (controller) {
446434
controller->watcher()->OnMachMessageReceived(port);
447435
}
436+
} else if (event->filter == EVFILT_TIMER) {
437+
// The wakeup timer fired.
438+
DCHECK_LE(scheduled_wakeup_time_, base::TimeTicks::Now());
439+
DCHECK_NE(scheduled_wakeup_time_, base::TimeTicks::Max());
440+
scheduled_wakeup_time_ = base::TimeTicks::Max();
441+
--event_count_;
448442
} else {
449443
NOTREACHED() << "Unexpected event for filter " << event->filter;
450444
}
@@ -453,4 +447,47 @@ bool MessagePumpKqueue::ProcessEvents(int count) {
453447
return did_work;
454448
}
455449

450+
void MessagePumpKqueue::UpdateWakeupTimer(const base::TimeTicks& wakeup_time) {
451+
DCHECK_NE(wakeup_time, scheduled_wakeup_time_);
452+
453+
// The ident of the wakeup timer. There's only the one timer as the pair
454+
// (ident, filter) is the identity of the event.
455+
constexpr uint64_t kWakeupTimerIdent = 0x0;
456+
if (wakeup_time == base::TimeTicks::Max()) {
457+
// Clear the timer.
458+
kevent64_s timer{};
459+
timer.ident = kWakeupTimerIdent;
460+
timer.filter = EVFILT_TIMER;
461+
timer.flags = EV_DELETE;
462+
463+
int rv = ChangeOneEvent(kqueue_, &timer);
464+
PCHECK(rv == 0) << "kevent64, delete timer";
465+
--event_count_;
466+
} else {
467+
// Set/reset the timer.
468+
kevent64_s timer{};
469+
timer.ident = kWakeupTimerIdent;
470+
timer.filter = EVFILT_TIMER;
471+
// This updates the timer if it already exists in |kqueue_|.
472+
timer.flags = EV_ADD | EV_ONESHOT;
473+
// Specify the sleep in microseconds to avoid undersleeping due to
474+
// numeric problems. The sleep is computed from TimeTicks::Now rather than
475+
// NextWorkInfo::recent_now because recent_now is strictly earlier than
476+
// current wall-clock. Using an earlier wall clock time to compute the
477+
// delta to the next wakeup wall-clock time would guarantee oversleep.
478+
// If wakeup_time is in the past, the delta below will be negative and the
479+
// timer is set immediately.
480+
timer.fflags = NOTE_USECONDS;
481+
timer.data = (wakeup_time - base::TimeTicks::Now()).InMicroseconds();
482+
int rv = ChangeOneEvent(kqueue_, &timer);
483+
PCHECK(rv == 0) << "kevent64, set timer";
484+
485+
// Bump the event count if we just added the timer.
486+
if (scheduled_wakeup_time_ == base::TimeTicks::Max())
487+
++event_count_;
488+
}
489+
490+
scheduled_wakeup_time_ = wakeup_time;
491+
}
492+
456493
} // namespace base

base/message_loop/message_pump_kqueue.h

+8
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,10 @@ class BASE_EXPORT MessagePumpKqueue : public MessagePump,
136136
// true if work was done, or false if no work was done.
137137
bool ProcessEvents(int count);
138138

139+
// Sets the wakeup timer to |wakeup_time|, or clears it if |wakeup_time| is
140+
// base::TimeTicks::Max(). Updates |scheduled_wakeup_time_| to follow.
141+
void UpdateWakeupTimer(const base::TimeTicks& wakeup_time);
142+
139143
// Receive right to which an empty Mach message is sent to wake up the pump
140144
// in response to ScheduleWork().
141145
mac::ScopedMachReceiveRight wakeup_;
@@ -159,6 +163,10 @@ class BASE_EXPORT MessagePumpKqueue : public MessagePump,
159163
// Whether the pump has been Quit() or not.
160164
bool keep_running_ = true;
161165

166+
// The currently scheduled wakeup, if any. If no wakeup is scheduled,
167+
// contains base::TimeTicks::Max().
168+
base::TimeTicks scheduled_wakeup_time_{base::TimeTicks::Max()};
169+
162170
// The number of events scheduled on the |kqueue_|. There is always at least
163171
// 1, for the |wakeup_| port (or |port_set_|).
164172
size_t event_count_ = 1;

0 commit comments

Comments
 (0)