From 2ad543758a214235d34df78f7389f44356046404 Mon Sep 17 00:00:00 2001 From: David Finkel Date: Mon, 13 Jan 2025 17:21:47 -0500 Subject: [PATCH 1/3] lru: use weak pointers in LRU stack Galaxycache isn't always the most friendly to the Go Garbage Collector. By keeping a whole pile of pointers in a linked list that the GC needs to scan in its mark phase, we're compromising the tail latency of any processes that maintain large numbers of objects in their caches. Fortunately, Go 1.24 provides weak pointers, so as long as we keep an "owning" reference in the cache's map, we can make all next/previous pointers weak pointers and avoid a whole bunch of extra GC work in its mark phase. (note: this optimization requires that the weak pointers be at the end of the struct so the GC can skip scanning them if there are pointers in the value -- if there aren't, then the GC can skip the linked list entries entirely -- not likely unless one has integer keys) --- lru/typed_ll.go | 2 +- lru/typed_ll_weak.go | 172 +++++++++++++++++++++++++++++++++++++++++++ lru/typed_lru.go | 4 + 3 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 lru/typed_ll_weak.go diff --git a/lru/typed_ll.go b/lru/typed_ll.go index 717cc039..8b29a9d7 100644 --- a/lru/typed_ll.go +++ b/lru/typed_ll.go @@ -1,4 +1,4 @@ -//go:build go1.18 +//go:build go1.18 && !go1.24 /* Copyright 2022 Vimeo Inc. diff --git a/lru/typed_ll_weak.go b/lru/typed_ll_weak.go new file mode 100644 index 00000000..2371d3a3 --- /dev/null +++ b/lru/typed_ll_weak.go @@ -0,0 +1,172 @@ +//go:build go1.24 + +/* +Copyright 2025 Vimeo Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package lru + +import ( + "fmt" + "weak" +) + +// Default-disable paranoid checks so they get compiled out. +// If this const is renamed/moved/updated make sure to update the sed +// expression in the github action. (.github/workflows/go.yml) +const paranoidLL = false + +// LinkedList using generics to reduce the number of heap objects +// Used for the LRU stack in TypedCache +// This implementation switches to using weak pointers when using go 1.24+ so +// the GC can skip scanning the linked list elements themselves. +type linkedList[T any] struct { + head weak.Pointer[llElem[T]] + tail weak.Pointer[llElem[T]] + size int +} + +type llElem[T any] struct { + value T + next, prev weak.Pointer[llElem[T]] +} + +func (l *llElem[T]) Next() *llElem[T] { + return l.next.Value() +} + +func (l *llElem[T]) Prev() *llElem[T] { + return l.prev.Value() +} + +func (l *linkedList[T]) PushFront(val T) *llElem[T] { + if paranoidLL { + l.checkHeadTail() + defer l.checkHeadTail() + } + elem := llElem[T]{ + next: l.head, + prev: weak.Pointer[llElem[T]]{}, // first element + value: val, + } + weakElem := weak.Make(&elem) + if lHead := l.head.Value(); lHead != nil { + lHead.prev = weakElem + } + if lTail := l.tail.Value(); lTail == nil { + l.tail = weakElem + } + l.head = weakElem + l.size++ + + return &elem +} + +func (l *linkedList[T]) MoveToFront(e *llElem[T]) { + if paranoidLL { + if e == nil { + panic("nil element") + } + l.checkHeadTail() + defer l.checkHeadTail() + } + + extHead := l.head.Value() + + if extHead == e { + // nothing to do + return + } + eWeak := weak.Make(e) + + if eNext := e.next.Value(); eNext != nil { + // update the previous pointer on the next element + eNext.prev = e.prev + } + if ePrev := e.prev.Value(); ePrev != nil { + ePrev.next = e.next + } + if lHead := l.head.Value(); lHead != nil { + lHead.prev = eWeak + } + + if lTail := l.tail.Value(); lTail == e { + l.tail = e.prev + } + e.next = l.head + l.head = eWeak + e.prev = weak.Pointer[llElem[T]]{} +} + +func (l *linkedList[T]) checkHeadTail() { + if !paranoidLL { + return + } + if (l.head.Value() != nil) != (l.tail.Value() != nil) { + panic(fmt.Sprintf("invariant failure; nilness mismatch: head: %+v; tail: %+v (size %d)", + l.head, l.tail, l.size)) + } + + if l.size > 0 && (l.head.Value() == nil || l.tail.Value() == nil) { + panic(fmt.Sprintf("invariant failure; head and/or tail nil with %d size: head: %+v; tail: %+v", + l.size, l.head, l.tail)) + } + + if lHead := l.head.Value(); lHead != nil && (lHead.prev.Value() != nil || (lHead.next.Value() == nil && l.size != 1)) { + panic(fmt.Sprintf("invariant failure; head next/prev invalid with %d size: head: %+v; tail: %+v", + l.size, l.head, l.tail)) + } + if lTail := l.tail.Value(); lTail != nil && ((lTail.prev.Value() == nil && l.size != 1) || lTail.next.Value() != nil) { + panic(fmt.Sprintf("invariant failure; tail next/prev invalid with %d size: head: %+v; tail: %+v", + l.size, l.head, l.tail)) + } +} + +func (l *linkedList[T]) Remove(e *llElem[T]) { + if paranoidLL { + if e == nil { + panic("nil element") + } + l.checkHeadTail() + defer l.checkHeadTail() + } + if l.tail.Value() == e { + l.tail = e.prev + } + if l.head.Value() == e { + l.head = e.next + } + + if eNext := e.next.Value(); eNext != nil { + // update the previous pointer on the next element + eNext.prev = e.prev + } + if ePrev := e.prev.Value(); ePrev != nil { + ePrev.next = e.next + } + l.size-- +} + +func (l *linkedList[T]) Len() int { + return l.size +} + +func (l *linkedList[T]) Front() *llElem[T] { + return l.head.Value() +} + +func (l *linkedList[T]) Back() *llElem[T] { + return l.tail.Value() +} diff --git a/lru/typed_lru.go b/lru/typed_lru.go index e596b1b1..f55aafe2 100644 --- a/lru/typed_lru.go +++ b/lru/typed_lru.go @@ -2,6 +2,7 @@ /* Copyright 2013 Google Inc. +Copyright 2022-2025 Vimeo Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -120,6 +121,9 @@ func (c *TypedCache[K, V]) RemoveOldest() { func (c *TypedCache[K, V]) removeElement(e *llElem[typedEntry[K, V]]) { c.ll.Remove(e) kv := e.value + // Wait until after we've removed the element from the linked list + // before removing from the map so we can leverage weak pointers in + // the linked list/LRU stack. delete(c.cache, kv.key) if c.OnEvicted != nil { c.OnEvicted(kv.key, kv.value) From 0bec1238c6c33835202e1c3e777add422383cfdf Mon Sep 17 00:00:00 2001 From: David Finkel Date: Tue, 11 Feb 2025 14:57:10 -0500 Subject: [PATCH 2/3] actions: add go 1.24 to the matrix --- .github/workflows/go.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 64e825e3..11ec8fbd 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -7,7 +7,7 @@ jobs: strategy: matrix: os: [macOS-latest, ubuntu-latest] - goversion: [1.17, 1.18, '1.19', '1.20', '1.21', '1.22', '1.23'] + goversion: [1.17, 1.18, '1.19', '1.20', '1.21', '1.22', '1.23', '1.24'] steps: - name: Set up Go ${{matrix.goversion}} on ${{matrix.os}} uses: actions/setup-go@v3 @@ -19,7 +19,7 @@ jobs: uses: actions/checkout@v1 - name: gofmt - if: ${{matrix.goversion == '1.23'}} + if: ${{matrix.goversion == '1.24'}} run: | [[ -z $(gofmt -l $(find . -name '*.go') ) ]] @@ -43,9 +43,9 @@ jobs: GO111MODULE: on run: go test -race -mod=readonly -count 2 ./... - # Run all consistenthash Fuzz tests for 30s with go 1.20 + # Run all consistenthash Fuzz tests for 30s with go 1.24 - name: Fuzz Consistent-Hash - if: ${{matrix.goversion == '1.20'}} + if: ${{matrix.goversion == '1.24'}} env: GO111MODULE: on run: go test -fuzz=. -fuzztime=30s ./consistenthash From f9cf640dccacd76d06cf2a765bbf4accad298b88 Mon Sep 17 00:00:00 2001 From: David Finkel Date: Tue, 11 Feb 2025 15:28:45 -0500 Subject: [PATCH 3/3] lru: add a fuzz test for adds/removes/lookup --- lru/typed_lru_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/lru/typed_lru_test.go b/lru/typed_lru_test.go index 18e84673..c0953871 100644 --- a/lru/typed_lru_test.go +++ b/lru/typed_lru_test.go @@ -2,6 +2,7 @@ /* Copyright 2013 Google Inc. +Copyright 2022-2025 Vimeo Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -21,6 +22,7 @@ package lru import ( "fmt" "testing" + "unicode/utf8" ) func TestTypedGet(t *testing.T) { @@ -92,6 +94,46 @@ func TestTypedEvict(t *testing.T) { } +func FuzzTypedAddRemove(f *testing.F) { + // Use a primitive bytecode scheme so the Fuzzer can add/remove/lookup objects pathwise and try to exercize as many states as possible + // we statically set the size to 16 entries to keep the state sane. + // "I" inserts the next key (with an empty value) up until the next recognized byte + // "D" deletes the next key (same as I -- up to the next recognized byte) + // "L" does a lookup for the next key (ditto) + // anything before a recognized command is ignored + f.Add("") + f.Add("IabcdLabcdDabcd") + f.Add("IabcLabcDabcIabcdLabcdDabcd") + f.Fuzz(func(t *testing.T, a string) { + l := TypedNew[string, struct{}](16) + + cmd := ' ' + keyStart := 0 + for o, r := range a { + // skip invalid runes + if !utf8.ValidRune(r) { + continue + } + switch r { + case 'I', 'D', 'L': + key := a[keyStart:o] + switch cmd { + case 'I': + l.Add(key, struct{}{}) + case 'D': + l.Remove(key) + case 'L': + l.Get(key) + } + cmd = r + // we need to start with the next rune + keyStart = o + utf8.RuneLen(r) + default: + } + } + }) +} + func BenchmarkTypedGetAllHits(b *testing.B) { b.ReportAllocs() type complexStruct struct {