Skip to content

Commit 3d9b997

Browse files
committed
Use a btree, it is faster
``` $ benchstat slice.txt btree.txt goos: darwin goarch: arm64 pkg: sigs.k8s.io/controller-runtime/pkg/controllerworkqueue cpu: Apple M2 Pro │ slice.txt │ btree.txt │ │ sec/op │ sec/op vs base │ AddGetDone-10 5.078m ± 0% 1.163m ± 0% -77.09% (p=0.000 n=10) │ slice.txt │ btree.txt │ │ B/op │ B/op vs base │ AddGetDone-10 55.11Ki ± 0% 46.98Ki ± 0% -14.75% (p=0.000 n=10) │ slice.txt │ btree.txt │ │ allocs/op │ allocs/op vs base │ AddGetDone-10 3.000k ± 0% 1.000k ± 0% -66.67% (p=0.000 n=10) ```
1 parent 2ed6574 commit 3d9b997

File tree

6 files changed

+64
-64
lines changed

6 files changed

+64
-64
lines changed

.gomodcheck.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,6 @@ excludedModules:
1212
# --- test dependencies:
1313
- github.com/onsi/ginkgo/v2
1414
- github.com/onsi/gomega
15+
16+
# --- We want a newer version with generics support for this
17+
- github.com/google/btree

examples/scratch-env/go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ require (
2323
github.com/go-openapi/swag v0.23.0 // indirect
2424
github.com/gogo/protobuf v1.3.2 // indirect
2525
github.com/golang/protobuf v1.5.4 // indirect
26+
github.com/google/btree v1.1.3 // indirect
2627
github.com/google/gnostic-models v0.6.8 // indirect
2728
github.com/google/go-cmp v0.6.0 // indirect
2829
github.com/google/gofuzz v1.2.0 // indirect

examples/scratch-env/go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
3535
github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q=
3636
github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=
3737
github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps=
38+
github.com/google/btree v1.1.3 h1:CVpQJjYgC4VbzxeGVHfvZrv1ctoYCAI8vbl07Fcxlyg=
39+
github.com/google/btree v1.1.3/go.mod h1:qOPhT0dTNdNzV6Z/lhRX0YXUafgPLFUh+gZMl761Gm4=
3840
github.com/google/gnostic-models v0.6.8 h1:yo/ABAfM5IMRsS1VnXjTBvUb61tFIHozhlYvRgGre9I=
3941
github.com/google/gnostic-models v0.6.8/go.mod h1:5n7qKqH0f5wFt+aWF8CW6pZLLNOfYuF5OpfBSENuI8U=
4042
github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=

go.mod

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ require (
3131
sigs.k8s.io/yaml v1.4.0
3232
)
3333

34+
require github.com/google/btree v1.1.3
35+
3436
require (
3537
github.com/antlr4-go/antlr/v4 v4.13.0 // indirect
3638
github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a // indirect

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
4949
github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q=
5050
github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=
5151
github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps=
52+
github.com/google/btree v1.1.3 h1:CVpQJjYgC4VbzxeGVHfvZrv1ctoYCAI8vbl07Fcxlyg=
53+
github.com/google/btree v1.1.3/go.mod h1:qOPhT0dTNdNzV6Z/lhRX0YXUafgPLFUh+gZMl761Gm4=
5254
github.com/google/cel-go v0.21.0 h1:cl6uW/gxN+Hy50tNYvI691+sXxioCnstFzLp2WO4GCI=
5355
github.com/google/cel-go v0.21.0/go.mod h1:rHUlWCcBKgyEk+eV03RPdZUekPp6YcJwV0FxuUksYxc=
5456
github.com/google/gnostic-models v0.6.8 h1:yo/ABAfM5IMRsS1VnXjTBvUb61tFIHozhlYvRgGre9I=

pkg/controllerworkqueue/workqueue.go

Lines changed: 54 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
package controllerworkqueue
22

33
import (
4-
"sort"
54
"sync"
65
"sync/atomic"
76
"time"
87

8+
"github.com/google/btree"
99
"k8s.io/apimachinery/pkg/util/sets"
1010
"k8s.io/client-go/util/workqueue"
1111
"k8s.io/utils/clock"
@@ -58,7 +58,7 @@ func New[T comparable](name string, o ...Opt[T]) PriorityQueue[T] {
5858

5959
cwq := &controllerworkqueue[T]{
6060
items: map[T]*item[T]{},
61-
queue: queue[T]{},
61+
queue: btree.NewG(32, less[T]),
6262
tryPush: make(chan struct{}, 1),
6363
rateLimiter: opts.RateLimiter,
6464
locked: sets.Set[T]{},
@@ -77,12 +77,16 @@ type controllerworkqueue[T comparable] struct {
7777
// lock has to be acquired for any access to either items or queue
7878
lock sync.Mutex
7979
items map[T]*item[T]
80-
queue queue[T]
80+
queue *btree.BTreeG[*item[T]]
8181

8282
tryPush chan struct{}
8383

8484
rateLimiter workqueue.TypedRateLimiter[T]
8585

86+
// addedCounter is a counter of elements added, we need it
87+
// because unixNano is not guaranteed to be unique.
88+
addedCounter uint64
89+
8690
// locked contains the keys we handed out through Get() and that haven't
8791
// yet been returned through Done().
8892
locked sets.Set[T]
@@ -106,7 +110,6 @@ func (w *controllerworkqueue[T]) AddWithOpts(o AddOpts, items ...T) {
106110
w.lock.Lock()
107111
defer w.lock.Unlock()
108112

109-
var hadChanges bool
110113
for _, key := range items {
111114
if o.RateLimited {
112115
after := w.rateLimiter.When(key)
@@ -121,30 +124,30 @@ func (w *controllerworkqueue[T]) AddWithOpts(o AddOpts, items ...T) {
121124
}
122125
if _, ok := w.items[key]; !ok {
123126
item := &item[T]{
124-
key: key,
125-
priority: o.Priority,
126-
readyAt: readyAt,
127+
key: key,
128+
addedAtUnixNano: w.now().UnixNano(),
129+
addedCounter: w.addedCounter,
130+
priority: o.Priority,
131+
readyAt: readyAt,
127132
}
128133
w.items[key] = item
129-
w.queue = append(w.queue, item)
130-
hadChanges = true
134+
w.queue.ReplaceOrInsert(item)
135+
w.addedCounter++
131136
continue
132137
}
133138

134-
if o.Priority > w.items[key].priority {
135-
w.items[key].priority = o.Priority
136-
hadChanges = true
139+
// The b-tree de-duplicates based on ordering and any change here
140+
// will affect the order - Just delete and re-add.
141+
item, _ := w.queue.Delete(w.items[key])
142+
if o.Priority > item.priority {
143+
item.priority = o.Priority
137144
}
138145

139-
if w.items[key].readyAt != nil && (readyAt == nil || readyAt.Before(*w.items[key].readyAt)) {
140-
w.items[key].readyAt = readyAt
141-
hadChanges = true
146+
if item.readyAt != nil && (readyAt == nil || readyAt.Before(*item.readyAt)) {
147+
item.readyAt = readyAt
142148
}
143-
}
144149

145-
if hadChanges {
146-
sort.Stable(w.queue)
147-
w.doTryPush()
150+
w.queue.ReplaceOrInsert(item)
148151
}
149152
}
150153

@@ -176,42 +179,30 @@ func (w *controllerworkqueue[T]) spin() {
176179
w.lockedLock.Lock()
177180
defer w.lockedLock.Unlock()
178181

179-
// toRemove is a list of indexes to remove from the queue.
180-
// We can not do it in-place as we would be manipulating the
181-
// slice we are iterating over. We have to do it backwards, as
182-
// otherwise the indexes become invalid.
183-
var toRemove []int
184-
defer func() {
185-
for i := len(toRemove) - 1; i >= 0; i-- {
186-
idxToRemove := toRemove[i]
187-
if idxToRemove == len(w.queue)-1 {
188-
w.queue = w.queue[:idxToRemove]
189-
} else {
190-
w.queue = append(w.queue[:idxToRemove], w.queue[idxToRemove+1:]...)
191-
}
192-
}
193-
}()
194-
for idx, item := range w.queue {
182+
w.queue.Ascend(func(item *item[T]) bool {
195183
if w.waiters.Load() == 0 { // no waiters, return as we can not hand anything out anyways
196-
return
184+
return false
197185
}
186+
198187
// No next element we can process
199-
if w.queue[0].readyAt != nil && w.queue[0].readyAt.After(w.now()) {
200-
nextReady = w.tick(w.queue[0].readyAt.Sub(w.now()))
201-
return
188+
if item.readyAt != nil && item.readyAt.After(w.now()) {
189+
nextReady = w.tick(item.readyAt.Sub(w.now()))
190+
return false
202191
}
203192

204193
// Item is locked, we can not hand it out
205194
if w.locked.Has(item.key) {
206-
continue
195+
return true
207196
}
208197

209198
w.get <- *item
210199
w.locked.Insert(item.key)
211-
delete(w.items, item.key)
212200
w.waiters.Add(-1)
213-
toRemove = append(toRemove, idx)
214-
}
201+
delete(w.items, item.key)
202+
w.queue.Delete(item)
203+
204+
return true
205+
})
215206
}()
216207
}
217208
}
@@ -274,37 +265,36 @@ func (w *controllerworkqueue[T]) Len() int {
274265
w.lock.Lock()
275266
defer w.lock.Unlock()
276267

277-
return len(w.queue)
268+
return w.queue.Len()
278269
}
279270

280-
// queue is the actual queue. It implements heap.Interface.
281-
type queue[T comparable] []*item[T]
282-
283-
func (q queue[T]) Len() int {
284-
return len(q)
285-
}
286-
287-
func (q queue[T]) Less(i, j int) bool {
288-
switch {
289-
case q[i].readyAt == nil && q[j].readyAt != nil:
271+
func less[T comparable](a, b *item[T]) bool {
272+
if a.readyAt == nil && b.readyAt != nil {
290273
return true
291-
case q[i].readyAt != nil && q[j].readyAt == nil:
274+
}
275+
if a.readyAt != nil && b.readyAt == nil {
292276
return false
293-
case q[i].readyAt != nil && q[j].readyAt != nil:
294-
return q[i].readyAt.Before(*q[j].readyAt)
277+
}
278+
if a.readyAt != nil && b.readyAt != nil && !a.readyAt.Equal(*b.readyAt) {
279+
return a.readyAt.Before(*b.readyAt)
280+
}
281+
if a.priority != b.priority {
282+
return a.priority > b.priority
295283
}
296284

297-
return q[i].priority > q[j].priority
298-
}
285+
if a.addedAtUnixNano != b.addedAtUnixNano {
286+
return a.addedAtUnixNano < b.addedAtUnixNano
287+
}
299288

300-
func (q queue[T]) Swap(i, j int) {
301-
q[i], q[j] = q[j], q[i]
289+
return a.addedCounter < b.addedCounter
302290
}
303291

304292
type item[T comparable] struct {
305-
key T
306-
priority int
307-
readyAt *time.Time
293+
key T
294+
addedAtUnixNano int64
295+
addedCounter uint64
296+
priority int
297+
readyAt *time.Time
308298
}
309299

310300
func wrapWithMetrics[T comparable](q *controllerworkqueue[T], name string, provider workqueue.MetricsProvider) PriorityQueue[T] {

0 commit comments

Comments
 (0)