perf(wayland): skip re-arming the pace timer for an unchanged deadline
- commit
- 3764080ca0fc7dc0d2ed36f813cf0445770f5b48
- author
- brice · · 2026-10-13 18:24 UTC
- committer
- brice · · 2026-10-14 04:34 UTC
- parent
- 470e764
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +98 -11
M internal/adapters/wayland/frames.go +34 -8
| @@ -41,20 +41,41 @@ func (s *Server) paceNow() { | |||
| 41 | 41 | s.paceTick(nil, nil) | |
| 42 | 42 | } | |
| 43 | 43 | ||
| 44 | + | // paceTolerance is how far a new deadline may differ from the armed one | |
| 45 | + | // and still reuse it: the ticks of one frame compute the same deadline from | |
| 46 | + | // clock reads a few microseconds apart. | |
| 47 | + | const paceTolerance = 50 * time.Microsecond | |
| 48 | + | ||
| 44 | 49 | // armPace schedules the next flip-less tick d from now on the pace timerfd | |
| 45 | - | // (nanosecond precision, one-shot). Display goroutine only. | |
| 50 | + | // (nanosecond precision, one-shot). A deadline equal to the armed one (within paceTolerance) keeps the timer as | |
| 51 | + | // it is: a frame's commit, flip and timer ticks mostly end on the same | |
| 52 | + | // deadline. The tolerance is measured against the armed deadline, so it never | |
| 53 | + | // accumulates. Display goroutine only. | |
| 46 | 54 | func (s *Server) armPace(d time.Duration) { | |
| 47 | - | if d <= 0 { | |
| 48 | - | d = time.Nanosecond | |
| 55 | + | deadline := time.Now().Add(max(d, 0)) | |
| 56 | + | if s.paceArmed { | |
| 57 | + | if diff := deadline.Sub(s.paceDeadline); diff >= -paceTolerance && diff <= paceTolerance { | |
| 58 | + | return | |
| 59 | + | } | |
| 49 | 60 | } | |
| 50 | - | spec := unix.ItimerSpec{Value: unix.NsecToTimespec(int64(d))} | |
| 51 | - | s.warnPace("arm", unix.TimerfdSettime(s.paceFD, 0, &spec, nil)) | |
| 61 | + | spec := unix.ItimerSpec{Value: unix.NsecToTimespec(int64(max(d, time.Nanosecond)))} | |
| 62 | + | err := unix.TimerfdSettime(s.paceFD, 0, &spec, nil) | |
| 63 | + | s.warnPace("arm", err) | |
| 64 | + | // A failed arm leaves the old state unknown: arm again next time. | |
| 65 | + | s.paceArmed, s.paceDeadline = err == nil, deadline | |
| 52 | 66 | } | |
| 53 | 67 | ||
| 54 | - | // stopPace disarms the pace timerfd. Display goroutine only. | |
| 68 | + | // stopPace disarms the pace timerfd, unless it is not armed (never armed, or | |
| 69 | + | // its one-shot expiry was consumed by onPaceTimer). Display goroutine only. | |
| 55 | 70 | func (s *Server) stopPace() { | |
| 71 | + | if !s.paceArmed { | |
| 72 | + | return | |
| 73 | + | } | |
| 56 | 74 | var spec unix.ItimerSpec | |
| 57 | - | s.warnPace("stop", unix.TimerfdSettime(s.paceFD, 0, &spec, nil)) | |
| 75 | + | err := unix.TimerfdSettime(s.paceFD, 0, &spec, nil) | |
| 76 | + | s.warnPace("stop", err) | |
| 77 | + | // A failed stop keeps the state: the next stop tries again. | |
| 78 | + | s.paceArmed = err != nil | |
| 58 | 79 | } | |
| 59 | 80 | ||
| 60 | 81 | // warnPace logs the first failure of a pace timerfd operation (later ones | |
| @@ -74,7 +95,12 @@ func (s *Server) onPaceTimer() { | |||
| 74 | 95 | var buf [8]byte | |
| 75 | 96 | // Level-triggered: consume the expiry. EAGAIN: the timer was disarmed | |
| 76 | 97 | // after the loop saw it readable. | |
| 77 | - | if _, err := unix.Read(s.paceFD, buf[:]); err != nil && !errors.Is(err, unix.EAGAIN) { | |
| 98 | + | n, err := unix.Read(s.paceFD, buf[:]) | |
| 99 | + | switch { | |
| 100 | + | case err == nil && n == len(buf): | |
| 101 | + | // The one-shot expired: the timer is disarmed until the next arm. | |
| 102 | + | s.paceArmed = false | |
| 103 | + | case err != nil && !errors.Is(err, unix.EAGAIN): | |
| 78 | 104 | s.warnPace("read", err) | |
| 79 | 105 | } | |
| 80 | 106 | s.paceTick(nil, nil) | |
| @@ -41,20 +41,41 @@ func (s *Server) paceNow() { | |||||
| 41 | s.paceTick(nil, nil) | 41 | s.paceTick(nil, nil) | ||
| 42 | } | 42 | } | ||
| 43 | 43 | ||||
| 44 | + | // paceTolerance is how far a new deadline may differ from the armed one | |||
| 45 | + | // and still reuse it: the ticks of one frame compute the same deadline from | |||
| 46 | + | // clock reads a few microseconds apart. | |||
| 47 | + | const paceTolerance = 50 * time.Microsecond | |||
| 48 | + | ||||
| 44 | // armPace schedules the next flip-less tick d from now on the pace timerfd | 49 | // armPace schedules the next flip-less tick d from now on the pace timerfd | ||
| 45 | - | // (nanosecond precision, one-shot). Display goroutine only. | 50 | + | // (nanosecond precision, one-shot). A deadline equal to the armed one (within paceTolerance) keeps the timer as |
| 51 | + | // it is: a frame's commit, flip and timer ticks mostly end on the same | |||
| 52 | + | // deadline. The tolerance is measured against the armed deadline, so it never | |||
| 53 | + | // accumulates. Display goroutine only. | |||
| 46 | func (s *Server) armPace(d time.Duration) { | 54 | func (s *Server) armPace(d time.Duration) { | ||
| 47 | - | if d <= 0 { | 55 | + | deadline := time.Now().Add(max(d, 0)) |
| 48 | - | d = time.Nanosecond | 56 | + | if s.paceArmed { |
| 57 | + | if diff := deadline.Sub(s.paceDeadline); diff >= -paceTolerance && diff <= paceTolerance { | |||
| 58 | + | return | |||
| 59 | + | } | |||
| 49 | } | 60 | } | ||
| 50 | - | spec := unix.ItimerSpec{Value: unix.NsecToTimespec(int64(d))} | 61 | + | spec := unix.ItimerSpec{Value: unix.NsecToTimespec(int64(max(d, time.Nanosecond)))} |
| 51 | - | s.warnPace("arm", unix.TimerfdSettime(s.paceFD, 0, &spec, nil)) | 62 | + | err := unix.TimerfdSettime(s.paceFD, 0, &spec, nil) |
| 63 | + | s.warnPace("arm", err) | |||
| 64 | + | // A failed arm leaves the old state unknown: arm again next time. | |||
| 65 | + | s.paceArmed, s.paceDeadline = err == nil, deadline | |||
| 52 | } | 66 | } | ||
| 53 | 67 | ||||
| 54 | - | // stopPace disarms the pace timerfd. Display goroutine only. | 68 | + | // stopPace disarms the pace timerfd, unless it is not armed (never armed, or |
| 69 | + | // its one-shot expiry was consumed by onPaceTimer). Display goroutine only. | |||
| 55 | func (s *Server) stopPace() { | 70 | func (s *Server) stopPace() { | ||
| 71 | + | if !s.paceArmed { | |||
| 72 | + | return | |||
| 73 | + | } | |||
| 56 | var spec unix.ItimerSpec | 74 | var spec unix.ItimerSpec | ||
| 57 | - | s.warnPace("stop", unix.TimerfdSettime(s.paceFD, 0, &spec, nil)) | 75 | + | err := unix.TimerfdSettime(s.paceFD, 0, &spec, nil) |
| 76 | + | s.warnPace("stop", err) | |||
| 77 | + | // A failed stop keeps the state: the next stop tries again. | |||
| 78 | + | s.paceArmed = err != nil | |||
| 58 | } | 79 | } | ||
| 59 | 80 | ||||
| 60 | // warnPace logs the first failure of a pace timerfd operation (later ones | 81 | // warnPace logs the first failure of a pace timerfd operation (later ones | ||
| @@ -74,7 +95,12 @@ func (s *Server) onPaceTimer() { | |||||
| 74 | var buf [8]byte | 95 | var buf [8]byte | ||
| 75 | // Level-triggered: consume the expiry. EAGAIN: the timer was disarmed | 96 | // Level-triggered: consume the expiry. EAGAIN: the timer was disarmed | ||
| 76 | // after the loop saw it readable. | 97 | // after the loop saw it readable. | ||
| 77 | - | if _, err := unix.Read(s.paceFD, buf[:]); err != nil && !errors.Is(err, unix.EAGAIN) { | 98 | + | n, err := unix.Read(s.paceFD, buf[:]) |
| 99 | + | switch { | |||
| 100 | + | case err == nil && n == len(buf): | |||
| 101 | + | // The one-shot expired: the timer is disarmed until the next arm. | |||
| 102 | + | s.paceArmed = false | |||
| 103 | + | case err != nil && !errors.Is(err, unix.EAGAIN): | |||
| 78 | s.warnPace("read", err) | 104 | s.warnPace("read", err) | ||
| 79 | } | 105 | } | ||
| 80 | s.paceTick(nil, nil) | 106 | s.paceTick(nil, nil) | ||
M internal/adapters/wayland/pace_inline_test.go +57 -0
| @@ -128,3 +128,60 @@ func TestSyncedCommitDoesNotTick(t *testing.T) { | |||
| 128 | 128 | t.Fatal("synced commit ticked the pacer") | |
| 129 | 129 | } | |
| 130 | 130 | } | |
| 131 | + | ||
| 132 | + | // An arm for the deadline already armed (within paceTolerance) keeps the | |
| 133 | + | // timer; another deadline re-arms it; an expiry consumed by onPaceTimer, or a | |
| 134 | + | // stop, leaves it disarmed, and a stop of a disarmed timer does nothing. | |
| 135 | + | func TestArmPaceSkipsUnchangedDeadline(t *testing.T) { | |
| 136 | + | s, _ := paceFixture(t, true) | |
| 137 | + | remaining := func() time.Duration { | |
| 138 | + | var cur unix.ItimerSpec | |
| 139 | + | if err := unix.TimerfdGettime(s.paceFD, &cur); err != nil { | |
| 140 | + | t.Fatal(err) | |
| 141 | + | } | |
| 142 | + | return time.Duration(cur.Value.Nano()) | |
| 143 | + | } | |
| 144 | + | s.stopPace() | |
| 145 | + | if s.paceArmed { | |
| 146 | + | t.Fatal("stop of a never armed timer armed it") | |
| 147 | + | } | |
| 148 | + | start := time.Now() | |
| 149 | + | s.armPace(time.Hour) | |
| 150 | + | first := s.paceDeadline | |
| 151 | + | if !s.paceArmed || remaining() <= 59*time.Minute { | |
| 152 | + | t.Fatalf("not armed: %v", remaining()) | |
| 153 | + | } | |
| 154 | + | // Later, with a wait that ends at the same moment (within tolerance): the | |
| 155 | + | // timer is kept. | |
| 156 | + | s.armPace(time.Hour - time.Since(start)) | |
| 157 | + | if !s.paceDeadline.Equal(first) { | |
| 158 | + | t.Fatalf("an unchanged deadline re-armed the timer: %v -> %v", first, s.paceDeadline) | |
| 159 | + | } | |
| 160 | + | s.armPace(time.Hour - time.Minute) | |
| 161 | + | if s.paceDeadline.Equal(first) || remaining() > 59*time.Minute { | |
| 162 | + | t.Fatalf("a new deadline kept the old timer: remaining %v", remaining()) | |
| 163 | + | } | |
| 164 | + | s.stopPace() | |
| 165 | + | if s.paceArmed || remaining() != 0 { | |
| 166 | + | t.Fatalf("stop left the timer armed: %v", remaining()) | |
| 167 | + | } | |
| 168 | + | // One-shot expiry: the fired timer is disarmed, so a stop is a no-op and | |
| 169 | + | // the next arm sets the timer again even for the same deadline. | |
| 170 | + | s.armPace(time.Nanosecond) | |
| 171 | + | fds := []unix.PollFd{{Fd: int32(s.paceFD), Events: unix.POLLIN}} | |
| 172 | + | if n, err := unix.Poll(fds, 5000); n != 1 { | |
| 173 | + | t.Fatalf("the timer did not fire: %d, %v", n, err) | |
| 174 | + | } | |
| 175 | + | s.onPaceTimer() | |
| 176 | + | if s.paceArmed { | |
| 177 | + | t.Fatal("expiry did not disarm") | |
| 178 | + | } | |
| 179 | + | s.stopPace() // no syscall to observe: it must not re-arm anything | |
| 180 | + | if remaining() != 0 { | |
| 181 | + | t.Fatalf("stop after the expiry: %v", remaining()) | |
| 182 | + | } | |
| 183 | + | s.armPace(time.Hour) | |
| 184 | + | if !s.paceArmed || remaining() <= 59*time.Minute { | |
| 185 | + | t.Fatalf("not re-armed after the expiry: %v", remaining()) | |
| 186 | + | } | |
| 187 | + | } | |
| @@ -128,3 +128,60 @@ func TestSyncedCommitDoesNotTick(t *testing.T) { | |||||
| 128 | t.Fatal("synced commit ticked the pacer") | 128 | t.Fatal("synced commit ticked the pacer") | ||
| 129 | } | 129 | } | ||
| 130 | } | 130 | } | ||
| 131 | + | ||||
| 132 | + | // An arm for the deadline already armed (within paceTolerance) keeps the | |||
| 133 | + | // timer; another deadline re-arms it; an expiry consumed by onPaceTimer, or a | |||
| 134 | + | // stop, leaves it disarmed, and a stop of a disarmed timer does nothing. | |||
| 135 | + | func TestArmPaceSkipsUnchangedDeadline(t *testing.T) { | |||
| 136 | + | s, _ := paceFixture(t, true) | |||
| 137 | + | remaining := func() time.Duration { | |||
| 138 | + | var cur unix.ItimerSpec | |||
| 139 | + | if err := unix.TimerfdGettime(s.paceFD, &cur); err != nil { | |||
| 140 | + | t.Fatal(err) | |||
| 141 | + | } | |||
| 142 | + | return time.Duration(cur.Value.Nano()) | |||
| 143 | + | } | |||
| 144 | + | s.stopPace() | |||
| 145 | + | if s.paceArmed { | |||
| 146 | + | t.Fatal("stop of a never armed timer armed it") | |||
| 147 | + | } | |||
| 148 | + | start := time.Now() | |||
| 149 | + | s.armPace(time.Hour) | |||
| 150 | + | first := s.paceDeadline | |||
| 151 | + | if !s.paceArmed || remaining() <= 59*time.Minute { | |||
| 152 | + | t.Fatalf("not armed: %v", remaining()) | |||
| 153 | + | } | |||
| 154 | + | // Later, with a wait that ends at the same moment (within tolerance): the | |||
| 155 | + | // timer is kept. | |||
| 156 | + | s.armPace(time.Hour - time.Since(start)) | |||
| 157 | + | if !s.paceDeadline.Equal(first) { | |||
| 158 | + | t.Fatalf("an unchanged deadline re-armed the timer: %v -> %v", first, s.paceDeadline) | |||
| 159 | + | } | |||
| 160 | + | s.armPace(time.Hour - time.Minute) | |||
| 161 | + | if s.paceDeadline.Equal(first) || remaining() > 59*time.Minute { | |||
| 162 | + | t.Fatalf("a new deadline kept the old timer: remaining %v", remaining()) | |||
| 163 | + | } | |||
| 164 | + | s.stopPace() | |||
| 165 | + | if s.paceArmed || remaining() != 0 { | |||
| 166 | + | t.Fatalf("stop left the timer armed: %v", remaining()) | |||
| 167 | + | } | |||
| 168 | + | // One-shot expiry: the fired timer is disarmed, so a stop is a no-op and | |||
| 169 | + | // the next arm sets the timer again even for the same deadline. | |||
| 170 | + | s.armPace(time.Nanosecond) | |||
| 171 | + | fds := []unix.PollFd{{Fd: int32(s.paceFD), Events: unix.POLLIN}} | |||
| 172 | + | if n, err := unix.Poll(fds, 5000); n != 1 { | |||
| 173 | + | t.Fatalf("the timer did not fire: %d, %v", n, err) | |||
| 174 | + | } | |||
| 175 | + | s.onPaceTimer() | |||
| 176 | + | if s.paceArmed { | |||
| 177 | + | t.Fatal("expiry did not disarm") | |||
| 178 | + | } | |||
| 179 | + | s.stopPace() // no syscall to observe: it must not re-arm anything | |||
| 180 | + | if remaining() != 0 { | |||
| 181 | + | t.Fatalf("stop after the expiry: %v", remaining()) | |||
| 182 | + | } | |||
| 183 | + | s.armPace(time.Hour) | |||
| 184 | + | if !s.paceArmed || remaining() <= 59*time.Minute { | |||
| 185 | + | t.Fatalf("not re-armed after the expiry: %v", remaining()) | |||
| 186 | + | } | |||
| 187 | + | } | |||
M internal/adapters/wayland/server.go +7 -3
| @@ -154,9 +154,13 @@ type Server struct { | |||
| 154 | 154 | // goroutine (paceReady: registered with the display loop); | |
| 155 | 155 | // paceDepth/paceDirty defer wakePacer inside a commit or tick (display | |
| 156 | 156 | // goroutine only). paceSignal coalesces posted ticks (paceSignalFn). | |
| 157 | - | paceFD int | |
| 158 | - | paceReady bool | |
| 159 | - | paceWarned bool // a pace timer failure was logged (once) | |
| 157 | + | paceFD int | |
| 158 | + | paceReady bool | |
| 159 | + | paceWarned bool // a pace timer failure was logged (once) | |
| 160 | + | // paceArmed: the timerfd is armed for paceDeadline (display goroutine | |
| 161 | + | // only); armPace and stopPace skip a syscall that would change nothing. | |
| 162 | + | paceArmed bool | |
| 163 | + | paceDeadline time.Time | |
| 160 | 164 | paceSignal atomic.Bool | |
| 161 | 165 | paceSignalFn func() | |
| 162 | 166 | paceDepth int | |
| @@ -154,9 +154,13 @@ type Server struct { | |||||
| 154 | // goroutine (paceReady: registered with the display loop); | 154 | // goroutine (paceReady: registered with the display loop); | ||
| 155 | // paceDepth/paceDirty defer wakePacer inside a commit or tick (display | 155 | // paceDepth/paceDirty defer wakePacer inside a commit or tick (display | ||
| 156 | // goroutine only). paceSignal coalesces posted ticks (paceSignalFn). | 156 | // goroutine only). paceSignal coalesces posted ticks (paceSignalFn). | ||
| 157 | - | paceFD int | 157 | + | paceFD int |
| 158 | - | paceReady bool | 158 | + | paceReady bool |
| 159 | - | paceWarned bool // a pace timer failure was logged (once) | 159 | + | paceWarned bool // a pace timer failure was logged (once) |
| 160 | + | // paceArmed: the timerfd is armed for paceDeadline (display goroutine | |||
| 161 | + | // only); armPace and stopPace skip a syscall that would change nothing. | |||
| 162 | + | paceArmed bool | |||
| 163 | + | paceDeadline time.Time | |||
| 160 | paceSignal atomic.Bool | 164 | paceSignal atomic.Bool | ||
| 161 | paceSignalFn func() | 165 | paceSignalFn func() | ||
| 162 | paceDepth int | 166 | paceDepth int | ||