fix(wayland): synced subsurface commits no longer tick the pacer

A synchronized child commit waits for its parent, so ticking for it only
cost CPU. forwardContents waits on contentReady instead of allocating a
notify channel per publication. The pacer timer is created only by Run,
and the inline pacer tests receive from the timer instead of racing Stop.

commit: 4d3f68196d254abec39c8a914b7fca279f0cfe6a
author: brice · 12d ago · 2026-10-10 14:48 UTC
committer: brice · 12d ago · 2026-10-10 15:39 UTC
parent: d17a46f
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

5 files · +36 -30

M internal/adapters/wayland/alloc_hotpath_test.go +1 -5


@@ -132,13 +132,9 @@ func TestEmitContentWakesWaiter(t *testing.T) {
132	132		damage: map[ports.WindowID][]ports.SeqDamage{},
133	133		contentReady: make(chan struct{}, 1),
134	134		}
135		-	s.contentMu.Lock()
136		-	s.contentNotify = make(chan struct{})
137		-	notify := s.contentNotify
138		-	s.contentMu.Unlock()
139	135		s.emitContent(ports.SurfaceContent{ID: 1, Solid: &ports.SolidColor{}}, damage{full: true})
140	136		select {
141		-	case <-notify:
	137	+	case <-s.contentReady:
142	138		default:
143	139		t.Fatal("waiter not woken")
144	140		}

 		damage:       map[ports.WindowID][]ports.SeqDamage{},
 		contentReady: make(chan struct{}, 1),
 	}
-	s.contentMu.Lock()
-	s.contentNotify = make(chan struct{})
-	notify := s.contentNotify
-	s.contentMu.Unlock()
 	s.emitContent(ports.SurfaceContent{ID: 1, Solid: &ports.SolidColor{}}, damage{full: true})
 	select {
-	case <-notify:
+	case <-s.contentReady:
 	default:
 		t.Fatal("waiter not woken")
 	}

M internal/adapters/wayland/frames.go +1 -4


@@ -33,10 +33,7 @@ const maxPacePasses = 3
33	33		// ticks, which all run there, so a commit that ticks inline (paceNow) never
34	34		// races the pacer for it.
35	35		func (s *Server) pace(ctx context.Context) {
36		-	if s.paceTimer == nil {
37		-	s.paceTimer = time.NewTimer(defaultFramePeriod)
38		-	}
39		-	timer := s.paceTimer
	36	+	timer := s.paceTimer // created by Run before the pacer starts
40	37		defer timer.Stop()
41	38		presented := s.channels.Presented
42	39		if s.flipped == nil {

 // ticks, which all run there, so a commit that ticks inline (paceNow) never
 // races the pacer for it.
 func (s *Server) pace(ctx context.Context) {
-	if s.paceTimer == nil {
+	timer := s.paceTimer // created by Run before the pacer starts
-		s.paceTimer = time.NewTimer(defaultFramePeriod)
-	}
-	timer := s.paceTimer
 	defer timer.Stop()
 	presented := s.channels.Presented
 	if s.flipped == nil {

M internal/adapters/wayland/pace_inline_test.go +27 -5


@@ -44,9 +44,7 @@ func TestCommitTicksPacerInline(t *testing.T) {
44	44		if len(s.frameReady) != 0 {
45	45		t.Fatal("commit woke the pacer goroutine")
46	46		}
47		-	if !s.paceTimer.Stop() {
48		-	t.Fatal("inline tick did not arm the pacer timer for the next deadline")
49		-	}
	47	+	waitArmed(t, s, "inline tick did not arm the pacer timer for the next deadline")
50	48		if s.paceDepth != 0 \|\| s.paceDirty {
51	49		t.Fatalf("pace state leaked: depth %d dirty %v", s.paceDepth, s.paceDirty)
52	50		}

@@ -87,7 +85,31 @@ func TestCommitWaitingOnBarrierArmsTimer(t *testing.T) {
87	85		if len(surf.queue) != 1 \|\| len(s.frameReady) != 0 {
88	86		t.Fatalf("queued %d, pacer wakes %d", len(surf.queue), len(s.frameReady))
89	87		}
90		-	if !s.paceTimer.Stop() {
91		-	t.Fatal("timer not armed for the barrier deadline")
	88	+	waitArmed(t, s, "timer not armed for the barrier deadline")
	89	+	}
	90	+
	91	+	// waitArmed asserts the timer was armed: it fires within the test timeout.
	92	+	// A receive works even when the deadline passed before the check.
	93	+	func waitArmed(t testing.T, s Server, msg string) {
	94	+	t.Helper()
	95	+	select {
	96	+	case <-s.paceTimer.C:
	97	+	case <-time.After(2 * time.Second):
	98	+	t.Fatal(msg)
	99	+	}
	100	+	}
	101	+
	102	+	// A synchronized subsurface commit waits for its parent's commit: it does
	103	+	// not tick the pacer, so a burst of child commits costs no ticks.
	104	+	func TestSyncedCommitDoesNotTick(t *testing.T) {
	105	+	s, surf := paceFixture(true)
	106	+	surf.sub.parent = &surface{server: s}
	107	+	surf.sub.synced = true
	108	+	surf.queueAndTick()
	109	+	if len(surf.queue) != 1 \|\| !surf.queue[0].synced {
	110	+	t.Fatalf("synced commit not queued: %d", len(surf.queue))
	111	+	}
	112	+	if len(s.frameReady) != 0 \|\| s.paceTimer.Stop() {
	113	+	t.Fatal("synced commit ticked the pacer")
92	114		}
93	115		}


@@ -44,9 +44,7 @@ func TestCommitTicksPacerInline(t *testing.T) {
44		if len(s.frameReady) != 0 {	44		if len(s.frameReady) != 0 {
45		t.Fatal("commit woke the pacer goroutine")	45		t.Fatal("commit woke the pacer goroutine")
46		}	46		}
47	-	if !s.paceTimer.Stop() {	47	+	waitArmed(t, s, "inline tick did not arm the pacer timer for the next deadline")
48	-	t.Fatal("inline tick did not arm the pacer timer for the next deadline")
49	-	}
50		if s.paceDepth != 0 \|\| s.paceDirty {	48		if s.paceDepth != 0 \|\| s.paceDirty {
51		t.Fatalf("pace state leaked: depth %d dirty %v", s.paceDepth, s.paceDirty)	49		t.Fatalf("pace state leaked: depth %d dirty %v", s.paceDepth, s.paceDirty)
52		}	50		}

@@ -87,7 +85,31 @@ func TestCommitWaitingOnBarrierArmsTimer(t *testing.T) {
87		if len(surf.queue) != 1 \|\| len(s.frameReady) != 0 {	85		if len(surf.queue) != 1 \|\| len(s.frameReady) != 0 {
88		t.Fatalf("queued %d, pacer wakes %d", len(surf.queue), len(s.frameReady))	86		t.Fatalf("queued %d, pacer wakes %d", len(surf.queue), len(s.frameReady))
89		}	87		}
90	-	if !s.paceTimer.Stop() {	88	+	waitArmed(t, s, "timer not armed for the barrier deadline")
91	-	t.Fatal("timer not armed for the barrier deadline")	89	+	}
			90	+
			91	+	// waitArmed asserts the timer was armed: it fires within the test timeout.
			92	+	// A receive works even when the deadline passed before the check.
			93	+	func waitArmed(t testing.T, s Server, msg string) {
			94	+	t.Helper()
			95	+	select {
			96	+	case <-s.paceTimer.C:
			97	+	case <-time.After(2 * time.Second):
			98	+	t.Fatal(msg)
			99	+	}
			100	+	}
			101	+
			102	+	// A synchronized subsurface commit waits for its parent's commit: it does
			103	+	// not tick the pacer, so a burst of child commits costs no ticks.
			104	+	func TestSyncedCommitDoesNotTick(t *testing.T) {
			105	+	s, surf := paceFixture(true)
			106	+	surf.sub.parent = &surface{server: s}
			107	+	surf.sub.synced = true
			108	+	surf.queueAndTick()
			109	+	if len(surf.queue) != 1 \|\| !surf.queue[0].synced {
			110	+	t.Fatalf("synced commit not queued: %d", len(surf.queue))
			111	+	}
			112	+	if len(s.frameReady) != 0 \|\| s.paceTimer.Stop() {
			113	+	t.Fatal("synced commit ticked the pacer")
92		}	114		}
93		}	115		}

M internal/adapters/wayland/server.go +4 -14


@@ -235,7 +235,6 @@ type Server struct {
235	235		clock ports.Clock
236	236		powers []*outputPower
237	237		outputsOff map[string]bool
238		-	contentNotify chan struct{}
239	238		contentReady chan struct{}
240	239		outputs []*output
241	240		outputManagers []*outputManager

@@ -666,12 +665,6 @@ func (s *Server) emitContent(c ports.SurfaceContent, d damage) {
666	665		}
667	666		c.DamageHistory = h[:len(h):len(h)]
668	667		s.contents[c.ID] = c
669		-	if s.contentNotify != nil {
670		-	// Only forwardContents waits on it, and it creates the channel when
671		-	// it does: no channel is allocated per commit.
672		-	close(s.contentNotify)
673		-	s.contentNotify = nil
674		-	}
675	668		s.contentMu.Unlock()
676	669		select {
677	670		case s.contentReady <- struct{}{}:

@@ -686,16 +679,11 @@ func (s *Server) forwardContents(ctx context.Context) {
686	679		var c ports.SurfaceContent
687	680		found := false
688	681		var seq uint64
689		-	var notify <-chan struct{}
690	682		for id, c = range s.contents {
691	683		found = true
692	684		seq = s.contentSeq[id]
693	685		break
694	686		}
695		-	if s.contentNotify == nil {
696		-	s.contentNotify = make(chan struct{})
697		-	}
698		-	notify = s.contentNotify
699	687		s.contentMu.Unlock()
700	688		if !found {
701	689		select {

@@ -705,11 +693,13 @@ func (s *Server) forwardContents(ctx context.Context) {
705	693		continue
706	694		}
707	695		}
708		-	// Do not hold a stale frame while the receiver is busy: wake on newer data.
	696	+	// Do not hold a stale frame while the receiver is busy: wake on newer
	697	+	// data. contentReady is signalled after every publication; a stale
	698	+	// signal only costs one more loop.
709	699		select {
710	700		case <-ctx.Done():
711	701		return
712		-	case <-notify:
	702	+	case <-s.contentReady:
713	703		continue
714	704		case s.channels.Contents <- c:
715	705		s.contentMu.Lock()

 	clock             ports.Clock
 	powers            []*outputPower
 	outputsOff        map[string]bool
-	contentNotify     chan struct{}
 	contentReady      chan struct{}
 	outputs           []*output
 	outputManagers    []*outputManager
 	}
 	c.DamageHistory = h[:len(h):len(h)]
 	s.contents[c.ID] = c
-	if s.contentNotify != nil {
-		// Only forwardContents waits on it, and it creates the channel when
-		// it does: no channel is allocated per commit.
-		close(s.contentNotify)
-		s.contentNotify = nil
-	}
 	s.contentMu.Unlock()
 	select {
 	case s.contentReady <- struct{}{}:
 		var c ports.SurfaceContent
 		found := false
 		var seq uint64
-		var notify <-chan struct{}
 		for id, c = range s.contents {
 			found = true
 			seq = s.contentSeq[id]
 			break
 		}
-		if s.contentNotify == nil {
-			s.contentNotify = make(chan struct{})
-		}
-		notify = s.contentNotify
 		s.contentMu.Unlock()
 		if !found {
 			select {
 				continue
 			}
 		}
-		// Do not hold a stale frame while the receiver is busy: wake on newer data.
+		// Do not hold a stale frame while the receiver is busy: wake on newer
+		// data. contentReady is signalled after every publication; a stale
+		// signal only costs one more loop.
 		select {
 		case <-ctx.Done():
 			return
-		case <-notify:
+		case <-s.contentReady:
 			continue
 		case s.channels.Contents <- c:
 			s.contentMu.Lock()

M internal/adapters/wayland/surface.go +3 -2


@@ -385,11 +385,12 @@ func (s *surface) queueAndTick() {
385	385		srv.paceDepth++
386	386		s.queueUpdate()
387	387		srv.tickFifo(time.Now(), nil)
388		-	if len(s.queue) > 0 {
	388	+	if n := len(s.queue); n > 0 && !s.queue[n-1].synced {
389	389		// The commit waits for a barrier, a fence or a refresh: the pacer
390	390		// must time it. A commit applied at once needs no tick of its own;
391	391		// its callbacks, barrier and held buffers wake the pacer where
392		-	// they are queued.
	392	+	// they are queued. A synchronized commit waits for its parent's
	393	+	// commit, which ticks for it.
393	394		srv.wakePacer()
394	395		}
395	396		srv.paceDepth--

 	srv.paceDepth++
 	s.queueUpdate()
 	srv.tickFifo(time.Now(), nil)
-	if len(s.queue) > 0 {
+	if n := len(s.queue); n > 0 && !s.queue[n-1].synced {
 		// The commit waits for a barrier, a fence or a refresh: the pacer
 		// must time it. A commit applied at once needs no tick of its own;
 		// its callbacks, barrier and held buffers wake the pacer where
-		// they are queued.
+		// they are queued. A synchronized commit waits for its parent's
+		// commit, which ticks for it.
 		srv.wakePacer()
 	}
 	srv.paceDepth--