perf(daemon): avoid inert coordinator timer allocations

commit: 778b92a4f2c3646df65b2a5f1751bb6b55ab2b71
author: brice · 3mo ago · 2026-07-11 18:08 UTC
parent: eca5562
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

2 files · +35 -22

M internal/usecase/daemon/render_coordinator.go +10 -8


@@ -211,6 +211,15 @@ func (c *renderCoordinator) invalidate(inv renderInvalidation) {
211	211		return
212	212		}
213	213		timer := clock.NewTimer(delay)
	214	+	timerC := timer.C()
	215	+	// A nil timer channel is inert, so it cannot need cancellation. Complete
	216	+	// synchronously before allocating a worker or its coordination channels.
	217	+	if timerC == nil {
	218	+	c.mu.Unlock()
	219	+	timer.Stop()
	220	+	c.fire(gen, false)
	221	+	return
	222	+	}
214	223		cancel := make(chan struct{})
215	224		done := make(chan struct{})
216	225		c.normalTimer, c.normalCancel, c.normalWorkerDone = timer, cancel, done

@@ -218,18 +227,11 @@ func (c *renderCoordinator) invalidate(inv renderInvalidation) {
218	227		go func() {
219	228		defer close(done)
220	229		select {
221		-	case <-timer.C():
	230	+	case <-timerC:
222	231		c.fire(gen, false)
223	232		case <-cancel:
224	233		}
225	234		}()
226		-	// A nil timer channel is an inert test clock, not a schedulable deadline.
227		-	// Complete through the coordinator so manual daemon fixtures retain their
228		-	// deterministic synchronous contract without a direct paint fallback.
229		-	if timer.C() == nil {
230		-	c.fire(gen, false)
231		-	return
232		-	}
233	235		// Give fake-clock callbacks a chance to block on C before the test
234	236		// advances it; real clocks are unaffected.
235	237		runtime.Gosched()

 		return
 	}
 	timer := clock.NewTimer(delay)
+	timerC := timer.C()
+	// A nil timer channel is inert, so it cannot need cancellation. Complete
+	// synchronously before allocating a worker or its coordination channels.
+	if timerC == nil {
+		c.mu.Unlock()
+		timer.Stop()
+		c.fire(gen, false)
+		return
+	}
 	cancel := make(chan struct{})
 	done := make(chan struct{})
 	c.normalTimer, c.normalCancel, c.normalWorkerDone = timer, cancel, done
 	go func() {
 		defer close(done)
 		select {
-		case <-timer.C():
+		case <-timerC:
 			c.fire(gen, false)
 		case <-cancel:
 		}
 	}()
-	// A nil timer channel is an inert test clock, not a schedulable deadline.
-	// Complete through the coordinator so manual daemon fixtures retain their
-	// deterministic synchronous contract without a direct paint fallback.
-	if timer.C() == nil {
-		c.fire(gen, false)
-		return
-	}
 	// Give fake-clock callbacks a chance to block on C before the test
 	// advances it; real clocks are unaffected.
 	runtime.Gosched()

M internal/usecase/daemon/render_coordinator_test.go +25 -14


@@ -814,7 +814,31 @@ func requireWorkerExit(t *testing.T, done <-chan struct{}) {
814	814		t.Fatal("cancelled coordinator timer worker did not exit")
815	815		}
816	816
817		-	func TestRenderCoordinatorStopsInertTimerWorkers(t *testing.T) {
	817	+	func TestRenderCoordinatorInertTimerFiresSynchronouslyWithoutWorker(t *testing.T) {
	818	+	clock := portsmocks.NewMockClock(t)
	819	+	timer := portsmocks.NewMockTimer(t)
	820	+	clock.EXPECT().NewTimer(minOutputRenderDeadline).Return(timer).Once()
	821	+	timer.EXPECT().C().Return((<-chan time.Time)(nil)).Once()
	822	+	timer.EXPECT().Stop().Return(true).Once()
	823	+
	824	+	wakes := make(chan renderWake, 1)
	825	+	rc := newRenderCoordinator(renderCoordinatorOptions{
	826	+	clock: clock,
	827	+	ackReady: func() bool { return true },
	828	+	wake: func(w renderWake) { wakes <- w },
	829	+	})
	830	+	rc.invalidate(renderInvalidation{class: invalidateOutput})
	831	+
	832	+	require.Equal(t, renderWake{coalesced: 1}, <-wakes)
	833	+	require.Equal(t, renderCoordinatorBurstMetricsSnapshot{invalidations: 1, wakes: 1, coalesced: 1}, rc.burstMetricsSnapshot())
	834	+	rc.mu.Lock()
	835	+	require.Nil(t, rc.normalTimer)
	836	+	require.Nil(t, rc.normalCancel)
	837	+	require.Nil(t, rc.normalWorkerDone)
	838	+	rc.mu.Unlock()
	839	+	}
	840	+
	841	+	func TestRenderCoordinatorStopsInertSyncTimerWorkers(t *testing.T) {
818	842		lifecycle := []struct {
819	843		name string
820	844		end func(renderCoordinator, attachedClient)

@@ -829,19 +853,6 @@ func TestRenderCoordinatorStopsInertTimerWorkers(t *testing.T) {
829	853		rc := newRenderCoordinator(renderCoordinatorOptions{clock: clk.clock})
830	854		ac := &attachedClient{}
831	855		rc.attach(ac)
832		-	rc.invalidate(renderInvalidation{class: invalidateOutput})
833		-	normal := <-clk.timers
834		-	rc.mu.Lock()
835		-	normalDone := rc.normalWorkerDone
836		-	rc.mu.Unlock()
837		-	tc.end(rc, ac)
838		-	normal.mock.AssertNumberOfCalls(t, "Stop", 1)
839		-	requireWorkerExit(t, normalDone)
840		-
841		-	// Use a fresh coordinator because detach/teardown intentionally makes
842		-	// an attachment ineligible for a new synchronized batch.
843		-	rc = newRenderCoordinator(renderCoordinatorOptions{clock: clk.clock})
844		-	rc.attach(ac)
845	856		rc.noteSyncBegin(1)
846	857		syncTimer := <-clk.timers
847	858		rc.mu.Lock()

 	t.Fatal("cancelled coordinator timer worker did not exit")
 }
-func TestRenderCoordinatorStopsInertTimerWorkers(t *testing.T) {
+func TestRenderCoordinatorInertTimerFiresSynchronouslyWithoutWorker(t *testing.T) {
+	clock := portsmocks.NewMockClock(t)
+	timer := portsmocks.NewMockTimer(t)
+	clock.EXPECT().NewTimer(minOutputRenderDeadline).Return(timer).Once()
+	timer.EXPECT().C().Return((<-chan time.Time)(nil)).Once()
+	timer.EXPECT().Stop().Return(true).Once()
+	wakes := make(chan renderWake, 1)
+	rc := newRenderCoordinator(renderCoordinatorOptions{
+		clock:    clock,
+		ackReady: func() bool { return true },
+		wake:     func(w renderWake) { wakes <- w },
+	})
+	rc.invalidate(renderInvalidation{class: invalidateOutput})
+	require.Equal(t, renderWake{coalesced: 1}, <-wakes)
+	require.Equal(t, renderCoordinatorBurstMetricsSnapshot{invalidations: 1, wakes: 1, coalesced: 1}, rc.burstMetricsSnapshot())
+	rc.mu.Lock()
+	require.Nil(t, rc.normalTimer)
+	require.Nil(t, rc.normalCancel)
+	require.Nil(t, rc.normalWorkerDone)
+	rc.mu.Unlock()
+}
+func TestRenderCoordinatorStopsInertSyncTimerWorkers(t *testing.T) {
 	lifecycle := []struct {
 		name string
 		end  func(*renderCoordinator, *attachedClient)
 			rc := newRenderCoordinator(renderCoordinatorOptions{clock: clk.clock})
 			ac := &attachedClient{}
 			rc.attach(ac)
-			rc.invalidate(renderInvalidation{class: invalidateOutput})
-			normal := <-clk.timers
-			rc.mu.Lock()
-			normalDone := rc.normalWorkerDone
-			rc.mu.Unlock()
-			tc.end(rc, ac)
-			normal.mock.AssertNumberOfCalls(t, "Stop", 1)
-			requireWorkerExit(t, normalDone)
-			// Use a fresh coordinator because detach/teardown intentionally makes
-			// an attachment ineligible for a new synchronized batch.
-			rc = newRenderCoordinator(renderCoordinatorOptions{clock: clk.clock})
-			rc.attach(ac)
 			rc.noteSyncBegin(1)
 			syncTimer := <-clk.timers
 			rc.mu.Lock()