Skip to content

Commit 6144fcb

Browse files
committed
perf(flowcontrol): index active queues so dispatch scans O(active) flows
Every dispatch cycle, the fairness policy's Pick calls IterateQueues, which snapshotted every registered queue in the band (empty ones included) into a freshly allocated slice under the registry read lock. Per-dispatch cost was O(registered flows) in CPU and allocation: 94-98% of all allocation in the flow-control benchmarks, and superlinear throughput decay under flow churn as registered flows accumulated between GC cycles. Each priority band now maintains a lock-free index of its non-empty queues, updated on empty<->non-empty transitions detected inside the queue's existing stats critical section (transitions are serialized per queue by the queue mutex; the index is a sync.Map, so the update never touches the registry mutex). IterateQueues ranges over the index directly: no registry lock, no allocation, cost proportional to flows that actually hold items. This narrows IterateQueues to active (non-empty) queues, matching its documented contract ("each active Flow"). Both production callers are compatible: globalstrict.Pick already skips empty queues, and the processor's cleanup/drain sweeps are no-ops on them. The view is eventually consistent; callbacks must tolerate a visited queue reporting Len() == 0, which they already did under the snapshot implementation. Benchmarks (M4 Pro, -benchtime=3s unless noted): - PerformanceMatrix L=1/P=1/F=5000/W=5000: 30.7k -> 277k d/s (9x), 42.2KB -> 1.8KB per op - TopologyChurn: 16.7k -> 260k d/s (15x), 83KB -> 1.8KB per op - FullPath (registry churn): 2.3k -> 118k d/s (52x); per-op cost now flat with run length (was superlinear: 106us at 24k flows minted -> 439us at 73k) Signed-off-by: Luke Van Drie <lukevandrie@google.com>
1 parent 04cc371 commit 6144fcb

5 files changed

Lines changed: 119 additions & 27 deletions

File tree

pkg/epp/flowcontrol/registry/managedqueue.go

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,13 @@ type managedQueue struct {
6363
// onStatsDelta is the callback used to propagate statistics changes up to the registry.
6464
onStatsDelta propagateStatsDeltaFunc
6565

66+
// onActiveTransition is invoked when the queue transitions between empty and non-empty, so the
67+
// owning priority band can maintain its index of active (non-empty) queues. Transitions are
68+
// detected under `mu`, which serializes them per queue. The callback must be lock-free or take
69+
// only leaf locks (never the registry mutex): it runs inside this queue's critical section.
70+
// May be nil (e.g., in isolated unit tests).
71+
onActiveTransition func(mq *managedQueue, active bool)
72+
6673
// --- State Protected by `mu` ---
6774

6875
// mu protects all mutating operations. It ensures that any changes to the underlying `queue` and the updates to the
@@ -92,14 +99,16 @@ func newManagedQueue(
9299
key flowcontrol.FlowKey,
93100
logger logr.Logger,
94101
onStatsDelta propagateStatsDeltaFunc,
102+
onActiveTransition func(mq *managedQueue, active bool),
95103
) *managedQueue {
96104
mqLogger := logger.WithName("managed-queue").WithValues("flowKey", key)
97105
mq := &managedQueue{
98-
queue: queue,
99-
policy: policy,
100-
key: key,
101-
onStatsDelta: onStatsDelta,
102-
logger: mqLogger,
106+
queue: queue,
107+
policy: policy,
108+
key: key,
109+
onStatsDelta: onStatsDelta,
110+
onActiveTransition: onActiveTransition,
111+
logger: mqLogger,
103112
}
104113
mq.flowQueueAccessor = &flowQueueAccessor{mq: mq}
105114
return mq
@@ -187,6 +196,17 @@ func (mq *managedQueue) propagateStatsDeltaLocked(lenDelta, byteSizeDelta int64)
187196
}
188197
mq.byteSize.Add(byteSizeDelta)
189198

199+
// Maintain the band's active-queue index on empty<->non-empty transitions. `mu` is held, so
200+
// transitions are strictly alternating per queue; `newLen == lenDelta` implies the previous
201+
// length was zero.
202+
if mq.onActiveTransition != nil && lenDelta != 0 {
203+
if lenDelta > 0 && newLen == lenDelta {
204+
mq.onActiveTransition(mq, true)
205+
} else if newLen == 0 {
206+
mq.onActiveTransition(mq, false)
207+
}
208+
}
209+
190210
// Propagate the delta up to the registry. This propagation is lock-free and eventually consistent.
191211
mq.onStatsDelta(mq.key.Priority, lenDelta, byteSizeDelta)
192212
}

pkg/epp/flowcontrol/registry/managedqueue_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func newMqHarness(t *testing.T, queue contracts.SafeQueue, key flowcontrol.FlowK
7070
propagator := &mockStatsPropagator{}
7171
mockPolicy := &fwkfcmocks.MockOrderingPolicy{}
7272

73-
mq := newManagedQueue(queue, mockPolicy, key, logr.Discard(), propagator.propagate)
73+
mq := newManagedQueue(queue, mockPolicy, key, logr.Discard(), propagator.propagate, nil)
7474
require.NotNil(t, mq, "Test setup: newManagedQueue must return a valid instance")
7575

7676
return &mqTestHarness{

pkg/epp/flowcontrol/registry/registry_helpers.go

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"fmt"
2222
"sort"
23+
"sync"
2324

2425
"github.com/llm-d/llm-d-router/pkg/common/observability/logging"
2526
"github.com/llm-d/llm-d-router/pkg/epp/flowcontrol/contracts"
@@ -50,6 +51,25 @@ type priorityBand struct {
5051

5152
// priorityBandAccessor is a preallocated flowcontrol.PriorityBandAccessor for this priorityBand
5253
priorityBandAccessor *priorityBandAccessor
54+
55+
// activeQueues indexes the subset of `queues` that currently hold items, keyed by logical ID
56+
// (values are *managedQueue). It is maintained by each queue's empty<->non-empty transitions
57+
// (serialized per queue under the queue's own mutex) and read lock-free by IterateQueues, which
58+
// keeps the dispatch hot path O(active flows) with zero allocation instead of O(registered
59+
// flows) with a snapshot. The view is eventually consistent: a queue is always present here by
60+
// the time an Add returns, but may linger briefly after draining; readers must tolerate
61+
// observing an empty queue.
62+
activeQueues sync.Map
63+
}
64+
65+
// setQueueActivity is the onActiveTransition callback for this band's queues. It runs inside the
66+
// queue's critical section, so it must remain lock-free (sync.Map only, never the registry mutex).
67+
func (b *priorityBand) setQueueActivity(mq *managedQueue, active bool) {
68+
if active {
69+
b.activeQueues.Store(mq.key.ID, mq)
70+
} else {
71+
b.activeQueues.Delete(mq.key.ID)
72+
}
5373
}
5474

5575
// initPriorityBand constructs the runtime state for a single priority level and registers it within the registry.
@@ -161,7 +181,7 @@ func (fr *FlowRegistry) synchronizeFlow(
161181

162182
fr.logger.V(logging.TRACE).Info("Creating new queue for flow instance.", "flowKey", key)
163183

164-
mq := newManagedQueue(q, policy, key, fr.logger, fr.propagateStatsDelta)
184+
mq := newManagedQueue(q, policy, key, fr.logger, fr.propagateStatsDelta, band.setQueueActivity)
165185
band.queues[key.ID] = mq
166186
}
167187

@@ -185,6 +205,7 @@ func (fr *FlowRegistry) deleteFlow(key flowcontrol.FlowKey) {
185205
}
186206
}
187207
delete(band.queues, key.ID)
208+
band.activeQueues.Delete(key.ID)
188209
}
189210
}
190211

@@ -244,22 +265,16 @@ func (a *priorityBandAccessor) Queue(id string) flowcontrol.FlowQueueAccessor {
244265
return mq.FlowQueueAccessor()
245266
}
246267

247-
// IterateQueues executes the given `callback` for each FlowQueueAccessor in this priority band.
268+
// IterateQueues executes the given `callback` for each active (non-empty) FlowQueueAccessor in
269+
// this priority band.
248270
//
249-
// To minimize lock contention, this implementation snapshots the queue accessors under a read lock and then executes
250-
// the callback on the snapshot, outside of the lock. This ensures that a potentially slow policy (the callback) does
251-
// not block other operations on the registry.
271+
// It ranges over the band's lock-free active-queue index, so it takes no registry lock and
272+
// performs no allocation, and its cost scales with the number of flows that currently hold items
273+
// rather than the number of registered flows. The view is eventually consistent: a queue drained
274+
// concurrently with iteration may still be visited, so callbacks must tolerate Len() == 0; a
275+
// queue is guaranteed to be visible once the Add that made it non-empty has returned.
252276
func (a *priorityBandAccessor) IterateQueues(callback func(queue flowcontrol.FlowQueueAccessor) bool) {
253-
a.registry.mu.RLock()
254-
accessors := make([]flowcontrol.FlowQueueAccessor, 0, len(a.band.queues))
255-
for _, mq := range a.band.queues {
256-
accessors = append(accessors, mq.FlowQueueAccessor())
257-
}
258-
a.registry.mu.RUnlock()
259-
260-
for _, accessor := range accessors {
261-
if !callback(accessor) {
262-
return
263-
}
264-
}
277+
a.band.activeQueues.Range(func(_, v any) bool {
278+
return callback(v.(*managedQueue).FlowQueueAccessor())
279+
})
265280
}

pkg/epp/flowcontrol/registry/registry_helpers_test.go

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -264,20 +264,71 @@ func TestRegistry_PriorityBandAccessor(t *testing.T) {
264264
t.Run("IterateQueues", func(t *testing.T) {
265265
t.Parallel()
266266

267-
t.Run("ShouldVisitAllQueuesInBand", func(t *testing.T) {
267+
t.Run("ShouldVisitAllActiveQueuesInBand", func(t *testing.T) {
268268
t.Parallel()
269+
h := newTestHarness(t) // Isolated harness: this subtest mutates queue contents.
270+
accessor, err := h.registry.PriorityBandAccessor(highPriority)
271+
require.NoError(t, err)
272+
h.addItem(h.highPriorityKey1, 100)
273+
h.addItem(h.highPriorityKey2, 100)
274+
269275
var iteratedKeys []flowcontrol.FlowKey
270276
accessor.IterateQueues(func(queue flowcontrol.FlowQueueAccessor) bool {
271277
iteratedKeys = append(iteratedKeys, queue.FlowKey())
272278
return true
273279
})
274280
expectedKeys := []flowcontrol.FlowKey{h.highPriorityKey1, h.highPriorityKey2}
275281
assert.ElementsMatch(t, expectedKeys, iteratedKeys,
276-
"IterateQueues must visit every registered flow in the band exactly once")
282+
"IterateQueues must visit every active (non-empty) flow in the band exactly once")
283+
})
284+
285+
t.Run("ShouldSkipEmptyQueues", func(t *testing.T) {
286+
t.Parallel()
287+
h := newTestHarness(t) // Isolated harness: this subtest mutates queue contents.
288+
accessor, err := h.registry.PriorityBandAccessor(highPriority)
289+
require.NoError(t, err)
290+
h.addItem(h.highPriorityKey1, 100)
291+
292+
var iteratedKeys []flowcontrol.FlowKey
293+
accessor.IterateQueues(func(queue flowcontrol.FlowQueueAccessor) bool {
294+
iteratedKeys = append(iteratedKeys, queue.FlowKey())
295+
return true
296+
})
297+
assert.Equal(t, []flowcontrol.FlowKey{h.highPriorityKey1}, iteratedKeys,
298+
"IterateQueues must visit only flows whose queues hold items; registered-but-empty flows are skipped")
299+
})
300+
301+
t.Run("ShouldTrackEmptinessTransitions", func(t *testing.T) {
302+
t.Parallel()
303+
h := newTestHarness(t) // Isolated harness: this subtest mutates queue contents.
304+
accessor, err := h.registry.PriorityBandAccessor(highPriority)
305+
require.NoError(t, err)
306+
307+
countVisited := func() int {
308+
var n int
309+
accessor.IterateQueues(func(queue flowcontrol.FlowQueueAccessor) bool {
310+
n++
311+
return true
312+
})
313+
return n
314+
}
315+
316+
item := h.addItem(h.highPriorityKey1, 100)
317+
assert.Equal(t, 1, countVisited(), "A flow must become visible once its queue holds an item")
318+
h.removeItem(h.highPriorityKey1, item)
319+
assert.Equal(t, 0, countVisited(), "A flow must stop being visited once its queue drains")
320+
h.addItem(h.highPriorityKey1, 100)
321+
assert.Equal(t, 1, countVisited(), "A drained flow must become visible again on re-add")
277322
})
278323

279324
t.Run("ShouldExitEarly_WhenCallbackReturnsFalse", func(t *testing.T) {
280325
t.Parallel()
326+
h := newTestHarness(t) // Isolated harness: this subtest mutates queue contents.
327+
accessor, err := h.registry.PriorityBandAccessor(highPriority)
328+
require.NoError(t, err)
329+
h.addItem(h.highPriorityKey1, 100)
330+
h.addItem(h.highPriorityKey2, 100)
331+
281332
var iterationCount int
282333
accessor.IterateQueues(func(queue flowcontrol.FlowQueueAccessor) bool {
283334
iterationCount++
@@ -307,12 +358,16 @@ func TestRegistry_PriorityBandAccessor(t *testing.T) {
307358
}
308359
}()
309360

310-
// Goroutine B: The Modifier (constantly writing)
361+
// Goroutine B: The Modifier (constantly writing). Item add/remove cycles exercise the
362+
// active-queue index transitions racing against iteration; flow create/delete cycles
363+
// exercise registry topology changes.
311364
go func() {
312365
defer wg.Done()
313366
for i := range 100 {
314367
key := flowcontrol.FlowKey{ID: fmt.Sprintf("new-flow-%d", i), Priority: highPriority}
315368
h.synchronizeFlow(key)
369+
item := h.addItem(key, 100)
370+
h.removeItem(key, item)
316371
h.registry.mu.Lock()
317372
h.registry.deleteFlow(key)
318373
h.registry.mu.Unlock()

pkg/epp/framework/interface/flowcontrol/accessors.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,9 @@ type PriorityBandAccessor interface {
6363
// Returns nil if the ID is not found in this group.
6464
Queue(id string) FlowQueueAccessor
6565

66-
// IterateQueues executes the given callback for each active Flow in this group.
66+
// IterateQueues executes the given callback for each active Flow in this group. A Flow is active while its queue
67+
// holds at least one item; implementations may skip empty queues entirely. The view may be eventually consistent
68+
// under concurrent mutation, so callbacks must tolerate a visited queue reporting Len() == 0.
6769
// Iteration stops if the callback returns false. The order of iteration is not guaranteed unless specified by
6870
// the implementation.
6971
IterateQueues(callback func(flow FlowQueueAccessor) (keepIterating bool))

0 commit comments

Comments
 (0)