Skip to content

Commit 5efcd18

Browse files
Meiri28clauderunatom-ai
committed
perf(rm): replace per-iteration sort in distributedAlloc with a min-heap
Follow-up to the tie-break fix in PR #1788. The previous implementation sorted the full candidate list inside the allocation loop, paying O(n log n) per iteration for n iterations and giving O(n² log n) overall. Since all annotated replicas from the same underlying physical device share the same sort key, sorting at the replica granularity is wasted work — only m (the number of distinct physical devices contributing candidates) needs to be reordered. Refactor to: - Bucket candidates by their underlying physical device into a small gpuAllocState per device, holding `used`, `pickedFrom`, and the remaining annotated-ID candidates from that device. - Initialize a min-heap of these states ordered primarily by `used` (so devices with the fewest already-allocated replicas come first) and tie-broken by `pickedFrom` (so devices we have not touched in the current allocation are preferred when used counts match). - On each iteration pop the best device, take one of its remaining replicas, increment its counters, and push it back if more remain. Total cost drops to O(n log m). The tie-break semantics from PR #1788 are preserved unchanged; existing tests still pass without modification. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-Authored-By: runatom-ai <258621014+runatom-ai@users.noreply.github.com> Signed-off-by: Jonathan Meiri <33288957+Meiri28@users.noreply.github.com>
1 parent 10fd1c0 commit 5efcd18

1 file changed

Lines changed: 67 additions & 45 deletions

File tree

internal/rm/allocate.go

Lines changed: 67 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,40 @@
1717
package rm
1818

1919
import (
20+
"container/heap"
2021
"fmt"
21-
"sort"
2222
)
2323

24+
// gpuAllocState holds per-physical-GPU bookkeeping for a single
25+
// distributedAlloc call.
26+
type gpuAllocState struct {
27+
used int // (total advertised) - (currently available to this allocation)
28+
pickedFrom int // slots picked from this device in the current allocation
29+
replicas []string // remaining annotated-ID candidates belonging to this device
30+
}
31+
32+
// gpuPriorityQueue is a min-heap of *gpuAllocState ordered primarily by
33+
// `used` so that devices with the fewest already-allocated replicas come
34+
// first, and tie-broken by `pickedFrom` so that devices we have not yet
35+
// touched during this allocation are preferred when used counts match.
36+
type gpuPriorityQueue []*gpuAllocState
37+
38+
func (q gpuPriorityQueue) Len() int { return len(q) }
39+
func (q gpuPriorityQueue) Less(i, j int) bool {
40+
if q[i].used != q[j].used {
41+
return q[i].used < q[j].used
42+
}
43+
return q[i].pickedFrom < q[j].pickedFrom
44+
}
45+
func (q gpuPriorityQueue) Swap(i, j int) { q[i], q[j] = q[j], q[i] }
46+
func (q *gpuPriorityQueue) Push(x any) { *q = append(*q, x.(*gpuAllocState)) }
47+
func (q *gpuPriorityQueue) Pop() any {
48+
n := len(*q) - 1
49+
x := (*q)[n]
50+
*q = (*q)[:n]
51+
return x
52+
}
53+
2454
// distributedAlloc returns a list of devices such that any replicated
2555
// devices are distributed across all replicated GPUs equally. It takes into
2656
// account already allocated replicas to ensure a proper balance across them.
@@ -33,60 +63,52 @@ func (r *resourceManager) distributedAlloc(available, required []string, size in
3363
return nil, fmt.Errorf("not enough available devices to satisfy allocation")
3464
}
3565

36-
// For each candidate device, build a mapping of (stripped) device ID to
37-
// total / available replicas for that device.
38-
replicas := make(map[string]*struct{ total, available int })
66+
// Bucket candidates by their underlying physical device and tally counts.
67+
// `used` is computed as (total replica records the plugin advertises for
68+
// this device) minus (the number of those records present in candidates).
69+
byGPU := make(map[string]*gpuAllocState)
3970
for _, c := range candidates {
4071
id := AnnotatedID(c).GetID()
41-
if _, exists := replicas[id]; !exists {
42-
replicas[id] = &struct{ total, available int }{}
72+
s, ok := byGPU[id]
73+
if !ok {
74+
s = &gpuAllocState{}
75+
byGPU[id] = s
4376
}
44-
replicas[id].available++
77+
s.replicas = append(s.replicas, c)
4578
}
4679
for d := range r.devices {
47-
id := AnnotatedID(d).GetID()
48-
if _, exists := replicas[id]; !exists {
49-
continue
80+
if s, ok := byGPU[AnnotatedID(d).GetID()]; ok {
81+
s.used++
5082
}
51-
replicas[id].total++
83+
}
84+
for _, s := range byGPU {
85+
s.used -= len(s.replicas)
5286
}
5387

54-
// Track how many slots have already been picked from each physical device
55-
// during this allocation. Used as the tie-break sort key below so the
56-
// allocator rotates to a sibling physical device when the underlying
57-
// "used" counts would otherwise tie.
58-
pickedFrom := make(map[string]int)
88+
// Build the priority queue once; subsequent picks reorder it in O(log m).
89+
pq := make(gpuPriorityQueue, 0, len(byGPU))
90+
for _, s := range byGPU {
91+
pq = append(pq, s)
92+
}
93+
heap.Init(&pq)
5994

60-
// Grab the set of 'needed' devices one-by-one from the candidates list.
61-
// Before selecting each candidate, first sort the candidate list using the
62-
// replicas map above. After sorting, the first element in the list will
63-
// contain the device with the least difference between total and available
64-
// replications (based on what's already been allocated). When two devices
65-
// tie on that count, prefer the physical device we have not touched (or
66-
// have touched the least) during this allocation. Add this device to the
67-
// list of devices to allocate, remove it from the candidate list, down
68-
// its available count in the replicas map, and repeat.
69-
var devices []string
95+
// Pop the highest-priority device, take one of its replicas, update its
96+
// counters, and push it back if more replicas remain. Total cost is
97+
// O(n log m), where n is `needed` and m is the number of distinct
98+
// physical devices contributing candidates.
99+
devices := make([]string, 0, needed)
70100
for i := 0; i < needed; i++ {
71-
sort.Slice(candidates, func(i, j int) bool {
72-
iid := AnnotatedID(candidates[i]).GetID()
73-
jid := AnnotatedID(candidates[j]).GetID()
74-
idiff := replicas[iid].total - replicas[iid].available
75-
jdiff := replicas[jid].total - replicas[jid].available
76-
if idiff != jdiff {
77-
return idiff < jdiff
78-
}
79-
return pickedFrom[iid] < pickedFrom[jid]
80-
})
81-
id := AnnotatedID(candidates[0]).GetID()
82-
pickedFrom[id]++
83-
replicas[id].available--
84-
devices = append(devices, candidates[0])
85-
candidates = candidates[1:]
101+
top := heap.Pop(&pq).(*gpuAllocState)
102+
last := len(top.replicas) - 1
103+
pick := top.replicas[last]
104+
top.replicas = top.replicas[:last]
105+
top.used++
106+
top.pickedFrom++
107+
if len(top.replicas) > 0 {
108+
heap.Push(&pq, top)
109+
}
110+
devices = append(devices, pick)
86111
}
87112

88-
// Add the set of required devices to this list and return it.
89-
devices = append(required, devices...)
90-
91-
return devices, nil
113+
return append(required, devices...), nil
92114
}

0 commit comments

Comments
 (0)