fix(wayland): never let a kept pace timer fire before the deadline
- commit
- 548f5e25059ce878d3428f2130e46eb7e073c819
- author
- brice · · 2026-10-13 18:33 UTC
- committer
- brice · · 2026-10-14 04:34 UTC
- parent
- 3764080
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +19 -11
M internal/adapters/wayland/frames.go +8 -6
| @@ -41,20 +41,22 @@ 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 | |
| 44 | + | // paceTolerance is how much earlier than the armed deadline a new one may be | |
| 45 | 45 | // and still reuse it: the ticks of one frame compute the same deadline from | |
| 46 | 46 | // clock reads a few microseconds apart. | |
| 47 | 47 | const paceTolerance = 50 * time.Microsecond | |
| 48 | 48 | ||
| 49 | 49 | // armPace schedules the next flip-less tick d from now on the pace timerfd | |
| 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. | |
| 50 | + | // (nanosecond precision, one-shot). A deadline at most paceTolerance before | |
| 51 | + | // the armed one keeps the timer as it is: a frame's commit, flip and timer | |
| 52 | + | // ticks mostly end on the same deadline. The timer never fires before the | |
| 53 | + | // requested deadline, since the tick's checks are strict and an early tick | |
| 54 | + | // would push due work back by their minimum re-arm. The tolerance is measured | |
| 55 | + | // against the armed deadline, so it never accumulates. Display goroutine only. | |
| 54 | 56 | func (s *Server) armPace(d time.Duration) { | |
| 55 | 57 | deadline := time.Now().Add(max(d, 0)) | |
| 56 | 58 | if s.paceArmed { | |
| 57 | - | if diff := deadline.Sub(s.paceDeadline); diff >= -paceTolerance && diff <= paceTolerance { | |
| 59 | + | if diff := deadline.Sub(s.paceDeadline); diff >= -paceTolerance && diff <= 0 { | |
| 58 | 60 | return | |
| 59 | 61 | } | |
| 60 | 62 | } | |
| @@ -41,20 +41,22 @@ 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 | 44 | + | // paceTolerance is how much earlier than the armed deadline a new one may be |
| 45 | // and still reuse it: the ticks of one frame compute the same deadline from | 45 | // and still reuse it: the ticks of one frame compute the same deadline from | ||
| 46 | // clock reads a few microseconds apart. | 46 | // clock reads a few microseconds apart. | ||
| 47 | const paceTolerance = 50 * time.Microsecond | 47 | const paceTolerance = 50 * time.Microsecond | ||
| 48 | 48 | ||||
| 49 | // 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 | ||
| 50 | - | // (nanosecond precision, one-shot). A deadline equal to the armed one (within paceTolerance) keeps the timer as | 50 | + | // (nanosecond precision, one-shot). A deadline at most paceTolerance before |
| 51 | - | // it is: a frame's commit, flip and timer ticks mostly end on the same | 51 | + | // the armed one keeps the timer as it is: a frame's commit, flip and timer |
| 52 | - | // deadline. The tolerance is measured against the armed deadline, so it never | 52 | + | // ticks mostly end on the same deadline. The timer never fires before the |
| 53 | - | // accumulates. Display goroutine only. | 53 | + | // requested deadline, since the tick's checks are strict and an early tick |
| 54 | + | // would push due work back by their minimum re-arm. The tolerance is measured | |||
| 55 | + | // against the armed deadline, so it never accumulates. Display goroutine only. | |||
| 54 | func (s *Server) armPace(d time.Duration) { | 56 | func (s *Server) armPace(d time.Duration) { | ||
| 55 | deadline := time.Now().Add(max(d, 0)) | 57 | deadline := time.Now().Add(max(d, 0)) | ||
| 56 | if s.paceArmed { | 58 | if s.paceArmed { | ||
| 57 | - | if diff := deadline.Sub(s.paceDeadline); diff >= -paceTolerance && diff <= paceTolerance { | 59 | + | if diff := deadline.Sub(s.paceDeadline); diff >= -paceTolerance && diff <= 0 { |
| 58 | return | 60 | return | ||
| 59 | } | 61 | } | ||
| 60 | } | 62 | } | ||
M internal/adapters/wayland/pace_inline_test.go +11 -5
| @@ -145,18 +145,24 @@ func TestArmPaceSkipsUnchangedDeadline(t *testing.T) { | |||
| 145 | 145 | if s.paceArmed { | |
| 146 | 146 | t.Fatal("stop of a never armed timer armed it") | |
| 147 | 147 | } | |
| 148 | - | start := time.Now() | |
| 149 | 148 | s.armPace(time.Hour) | |
| 150 | 149 | first := s.paceDeadline | |
| 151 | 150 | if !s.paceArmed || remaining() <= 59*time.Minute { | |
| 152 | 151 | t.Fatalf("not armed: %v", remaining()) | |
| 153 | 152 | } | |
| 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)) | |
| 153 | + | // A deadline a little before the armed one keeps the timer. A full | |
| 154 | + | // tolerance of margin absorbs the clock advancing between the calls. | |
| 155 | + | s.armPace(time.Until(first) - paceTolerance/2) | |
| 157 | 156 | if !s.paceDeadline.Equal(first) { | |
| 158 | - | t.Fatalf("an unchanged deadline re-armed the timer: %v -> %v", first, s.paceDeadline) | |
| 157 | + | t.Fatalf("an earlier deadline within tolerance re-armed the timer: %v -> %v", first, s.paceDeadline) | |
| 159 | 158 | } | |
| 159 | + | // A later deadline, however close, re-arms: the timer must never fire | |
| 160 | + | // before the requested deadline. | |
| 161 | + | s.armPace(time.Until(first) + time.Microsecond) | |
| 162 | + | if !s.paceDeadline.After(first) { | |
| 163 | + | t.Fatalf("a later deadline kept the earlier timer: %v -> %v", first, s.paceDeadline) | |
| 164 | + | } | |
| 165 | + | first = s.paceDeadline | |
| 160 | 166 | s.armPace(time.Hour - time.Minute) | |
| 161 | 167 | if s.paceDeadline.Equal(first) || remaining() > 59*time.Minute { | |
| 162 | 168 | t.Fatalf("a new deadline kept the old timer: remaining %v", remaining()) | |
| @@ -145,18 +145,24 @@ func TestArmPaceSkipsUnchangedDeadline(t *testing.T) { | |||||
| 145 | if s.paceArmed { | 145 | if s.paceArmed { | ||
| 146 | t.Fatal("stop of a never armed timer armed it") | 146 | t.Fatal("stop of a never armed timer armed it") | ||
| 147 | } | 147 | } | ||
| 148 | - | start := time.Now() | |||
| 149 | s.armPace(time.Hour) | 148 | s.armPace(time.Hour) | ||
| 150 | first := s.paceDeadline | 149 | first := s.paceDeadline | ||
| 151 | if !s.paceArmed || remaining() <= 59*time.Minute { | 150 | if !s.paceArmed || remaining() <= 59*time.Minute { | ||
| 152 | t.Fatalf("not armed: %v", remaining()) | 151 | t.Fatalf("not armed: %v", remaining()) | ||
| 153 | } | 152 | } | ||
| 154 | - | // Later, with a wait that ends at the same moment (within tolerance): the | 153 | + | // A deadline a little before the armed one keeps the timer. A full |
| 155 | - | // timer is kept. | 154 | + | // tolerance of margin absorbs the clock advancing between the calls. |
| 156 | - | s.armPace(time.Hour - time.Since(start)) | 155 | + | s.armPace(time.Until(first) - paceTolerance/2) |
| 157 | if !s.paceDeadline.Equal(first) { | 156 | if !s.paceDeadline.Equal(first) { | ||
| 158 | - | t.Fatalf("an unchanged deadline re-armed the timer: %v -> %v", first, s.paceDeadline) | 157 | + | t.Fatalf("an earlier deadline within tolerance re-armed the timer: %v -> %v", first, s.paceDeadline) |
| 159 | } | 158 | } | ||
| 159 | + | // A later deadline, however close, re-arms: the timer must never fire | |||
| 160 | + | // before the requested deadline. | |||
| 161 | + | s.armPace(time.Until(first) + time.Microsecond) | |||
| 162 | + | if !s.paceDeadline.After(first) { | |||
| 163 | + | t.Fatalf("a later deadline kept the earlier timer: %v -> %v", first, s.paceDeadline) | |||
| 164 | + | } | |||
| 165 | + | first = s.paceDeadline | |||
| 160 | s.armPace(time.Hour - time.Minute) | 166 | s.armPace(time.Hour - time.Minute) | ||
| 161 | if s.paceDeadline.Equal(first) || remaining() > 59*time.Minute { | 167 | if s.paceDeadline.Equal(first) || remaining() > 59*time.Minute { | ||
| 162 | t.Fatalf("a new deadline kept the old timer: remaining %v", remaining()) | 168 | t.Fatalf("a new deadline kept the old timer: remaining %v", remaining()) | ||