From 05e6715821a2da6513af5bd35dad2655acb6975a Mon Sep 17 00:00:00 2001 From: David Finkel Date: Tue, 2 Aug 2022 11:49:18 -0400 Subject: [PATCH 1/7] backportable microbenchmarks for Gets() Benchmarks for both the lru package and top-level package. --- galaxycache_test.go | 68 +++++++++++++++++++++++++++++++++++++++++++++ lru/lru_test.go | 19 +++++++++++++ 2 files changed, 87 insertions(+) diff --git a/galaxycache_test.go b/galaxycache_test.go index 15c97108..32d8dd3a 100644 --- a/galaxycache_test.go +++ b/galaxycache_test.go @@ -704,3 +704,71 @@ func TestRecorder(t *testing.T) { t.Errorf("expected 1 row, got %d", len(rows)) } } + +func BenchmarkGetsSerialOneKey(b *testing.B) { + b.ReportAllocs() + + ctx := context.Background() + + u := NewUniverse(&NullFetchProtocol{}, "test") + + const testKey = "somekey" + const testVal = "testval" + g := u.NewGalaxy("testgalaxy", 1024, GetterFunc(func(_ context.Context, key string, dest Codec) error { + return dest.UnmarshalBinary([]byte(testVal)) + })) + + cd := ByteCodec{} + b.ResetTimer() + + for z := 0; z < b.N; z++ { + g.Get(ctx, testKey, &cd) + } + +} + +func BenchmarkGetsSerialManyKeys(b *testing.B) { + b.ReportAllocs() + + ctx := context.Background() + + u := NewUniverse(&NullFetchProtocol{}, "test") + + const testVal = "testval" + g := u.NewGalaxy("testgalaxy", 1024, GetterFunc(func(_ context.Context, key string, dest Codec) error { + return dest.UnmarshalBinary([]byte(testVal)) + })) + + cd := ByteCodec{} + b.ResetTimer() + + for z := 0; z < b.N; z++ { + k := "zzzz" + strconv.Itoa(z&0xffff) + + g.Get(ctx, k, &cd) + } +} + +func BenchmarkGetsParallelManyKeys(b *testing.B) { + b.ReportAllocs() + + ctx := context.Background() + + u := NewUniverse(&NullFetchProtocol{}, "test") + + const testVal = "testval" + g := u.NewGalaxy("testgalaxy", 1024, GetterFunc(func(_ context.Context, key string, dest Codec) error { + return dest.UnmarshalBinary([]byte(testVal)) + })) + + cd := ByteCodec{} + b.ResetTimer() + + b.RunParallel(func(pb *testing.PB) { + for z := 0; pb.Next(); z++ { + k := "zzzz" + strconv.Itoa(z&0xffff) + + g.Get(ctx, k, &cd) + } + }) +} diff --git a/lru/lru_test.go b/lru/lru_test.go index 870fe38c..05983558 100644 --- a/lru/lru_test.go +++ b/lru/lru_test.go @@ -95,3 +95,22 @@ func TestEvict(t *testing.T) { t.Fatalf("got %v in second evicted key; want %s", evictedKeys[1], "myKey1") } } + +func BenchmarkGetAllHits(b *testing.B) { + b.ReportAllocs() + type complexStruct struct { + a, b, c, d, e, f int64 + k, l, m, n, o, p float64 + } + // Populate the cache + l := New(32) + for z := 0; z < 32; z++ { + l.Add(z, &complexStruct{a: int64(z)}) + } + + b.ResetTimer() + for z := 0; z < b.N; z++ { + // take the lower 5 bits as mod 32 so we always hit + l.Get(z & 31) + } +} From cb57458ac43795926d8972ee3f51acebcf3950ba Mon Sep 17 00:00:00 2001 From: David Finkel Date: Tue, 2 Aug 2022 11:49:56 -0400 Subject: [PATCH 2/7] Typed benchmarks for new lru implementation Not backportable --- lru/typed_lru_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/lru/typed_lru_test.go b/lru/typed_lru_test.go index f92ba45a..3dd45d45 100644 --- a/lru/typed_lru_test.go +++ b/lru/typed_lru_test.go @@ -83,3 +83,41 @@ func TestTypedEvict(t *testing.T) { t.Fatalf("got %v in second evicted key; want %s", evictedKeys[1], "myKey1") } } + +func BenchmarkTypedGetAllHits(b *testing.B) { + b.ReportAllocs() + type complexStruct struct { + a, b, c, d, e, f int64 + k, l, m, n, o, p float64 + } + // Populate the cache + l := TypedNew[int, complexStruct](32) + for z := 0; z < 32; z++ { + l.Add(z, complexStruct{a: int64(z)}) + } + + b.ResetTimer() + for z := 0; z < b.N; z++ { + // take the lower 5 bits as mod 32 so we always hit + l.Get(z & 31) + } +} + +func BenchmarkTypedGetHalfHits(b *testing.B) { + b.ReportAllocs() + type complexStruct struct { + a, b, c, d, e, f int64 + k, l, m, n, o, p float64 + } + // Populate the cache + l := TypedNew[int, complexStruct](32) + for z := 0; z < 32; z++ { + l.Add(z, complexStruct{a: int64(z)}) + } + + b.ResetTimer() + for z := 0; z < b.N; z++ { + // take the lower 4 bits as mod 16 shifted left by 1 to + l.Get((z&15)<<1 | z&16>>4 | z&1<<4) + } +} From 67b867373d592ed22cf924c9bd4907e38c53c67f Mon Sep 17 00:00:00 2001 From: David Finkel Date: Tue, 2 Aug 2022 12:37:23 -0400 Subject: [PATCH 3/7] benchmark: add benchmark with more goroutines --- galaxycache_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/galaxycache_test.go b/galaxycache_test.go index 32d8dd3a..fd98b922 100644 --- a/galaxycache_test.go +++ b/galaxycache_test.go @@ -24,6 +24,7 @@ import ( "fmt" "math" "math/rand" + "runtime" "strconv" "strings" "sync" @@ -772,3 +773,46 @@ func BenchmarkGetsParallelManyKeys(b *testing.B) { } }) } + +func BenchmarkGetsParallelManyKeysWithGoroutines(b *testing.B) { + // Traverse the powers of two + for mul := 1; mul < 128; mul <<= 1 { + b.Run(strconv.Itoa(mul), func(b *testing.B) { + b.ReportAllocs() + + ctx := context.Background() + + u := NewUniverse(&NullFetchProtocol{}, "test") + + const testVal = "testval" + g := u.NewGalaxy("testgalaxy", 1024, GetterFunc(func(_ context.Context, key string, dest Codec) error { + return dest.UnmarshalBinary([]byte(testVal)) + })) + + gmp := runtime.GOMAXPROCS(-1) + + grs := gmp * mul + + iters := b.N / grs + + wg := sync.WaitGroup{} + + b.ResetTimer() + + for gr := 0; gr < grs; gr++ { + wg.Add(1) + go func() { + defer wg.Done() + cd := ByteCodec{} + for z := 0; z < iters; z++ { + k := "zzzz" + strconv.Itoa(z&0xffff) + + g.Get(ctx, k, &cd) + } + }() + } + wg.Wait() + + }) + } +} From 40c328d9e35fcc2c02a8701d1b389271635973ae Mon Sep 17 00:00:00 2001 From: David Finkel Date: Tue, 2 Aug 2022 16:31:23 -0400 Subject: [PATCH 4/7] lru: linkedList: MoveToFront update next/yead/prev Fix an egregious invariant violation: set the element next and prev pointers correctly for the head element, and actually set the head pointer on the linked-list. --- lru/typed_ll.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lru/typed_ll.go b/lru/typed_ll.go index 5ee03f69..61c96e9c 100644 --- a/lru/typed_ll.go +++ b/lru/typed_ll.go @@ -77,6 +77,9 @@ func (l *linkedList[T]) MoveToFront(e *llElem[T]) { if l.tail == e { l.tail = e.prev } + e.next = l.head + l.head = e + e.prev = nil } func (l *linkedList[T]) Remove(e *llElem[T]) { From 11d4f52ed6882a87c06a9ab98dc442f718fdfa1c Mon Sep 17 00:00:00 2001 From: David Finkel Date: Tue, 2 Aug 2022 16:33:40 -0400 Subject: [PATCH 5/7] lru: paranoid linkedList checks Add checks for invariants that would be expensive at runtime, but are invaluable for checking invariants on the linkedlist implementation. Add a phase to the github actions run that enables the paranoid check. --- .github/workflows/go.yml | 9 ++++++++ lru/typed_ll.go | 50 ++++++++++++++++++++++++++++++++++++++++ lru/typed_lru_test.go | 8 +++++++ 3 files changed, 67 insertions(+) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 81d3c25f..c9ad9587 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -41,3 +41,12 @@ jobs: env: GO111MODULE: on run: go test -race -mod=readonly -count 2 ./... + + # Run all the tests with the paranoid linked-list checks enabled + - name: Race Test Paranoid LinkedList + env: + GO111MODULE: on + run: | + sed -e 's/const paranoidLL = false/const paranoidLL = true/' < lru/typed_ll.go > lru/typed_ll.go.new + mv lru/typed_ll.go.new lru/typed_ll.go + go test -race -mod=readonly -count 2 ./... diff --git a/lru/typed_ll.go b/lru/typed_ll.go index 61c96e9c..717cc039 100644 --- a/lru/typed_ll.go +++ b/lru/typed_ll.go @@ -18,6 +18,13 @@ limitations under the License. package lru +import "fmt" + +// 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 type linkedList[T any] struct { @@ -40,6 +47,10 @@ func (l *llElem[T]) Prev() *llElem[T] { } func (l *linkedList[T]) PushFront(val T) *llElem[T] { + if paranoidLL { + l.checkHeadTail() + defer l.checkHeadTail() + } elem := llElem[T]{ next: l.head, prev: nil, // first element @@ -58,6 +69,14 @@ func (l *linkedList[T]) PushFront(val T) *llElem[T] { } func (l *linkedList[T]) MoveToFront(e *llElem[T]) { + if paranoidLL { + if e == nil { + panic("nil element") + } + l.checkHeadTail() + defer l.checkHeadTail() + } + if l.head == e { // nothing to do return @@ -82,7 +101,38 @@ func (l *linkedList[T]) MoveToFront(e *llElem[T]) { e.prev = nil } +func (l *linkedList[T]) checkHeadTail() { + if !paranoidLL { + return + } + if (l.head != nil) != (l.tail != 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 == nil || l.tail == 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 l.head != nil && (l.head.prev != nil || (l.head.next == 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 l.tail != nil && ((l.tail.prev == nil && l.size != 1) || l.tail.next != 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 == e { l.tail = e.prev } diff --git a/lru/typed_lru_test.go b/lru/typed_lru_test.go index 3dd45d45..18e84673 100644 --- a/lru/typed_lru_test.go +++ b/lru/typed_lru_test.go @@ -82,6 +82,14 @@ func TestTypedEvict(t *testing.T) { if evictedKeys[1] != Key("myKey1") { t.Fatalf("got %v in second evicted key; want %s", evictedKeys[1], "myKey1") } + // move 9 and 10 to the head + lru.Get("myKey10") + lru.Get("myKey9") + // add another few keys to evict the the others + for i := 22; i < 32; i++ { + lru.Add(fmt.Sprintf("myKey%d", i), 1234) + } + } func BenchmarkTypedGetAllHits(b *testing.B) { From ce5caf66a319806e399bcc335cbcab5f855910a6 Mon Sep 17 00:00:00 2001 From: David Finkel Date: Tue, 2 Aug 2022 16:35:38 -0400 Subject: [PATCH 6/7] test: parallel test for verify concurrent behavior Add a tes that exercizes the concurrent behavior. --- galaxycache_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/galaxycache_test.go b/galaxycache_test.go index fd98b922..33c0b5af 100644 --- a/galaxycache_test.go +++ b/galaxycache_test.go @@ -774,6 +774,51 @@ func BenchmarkGetsParallelManyKeys(b *testing.B) { }) } +func TestGetsParallelManyKeysWithGoroutines(t *testing.T) { + if testing.Short() { + t.Skip("skipping in short mode") + } + t.Parallel() + const N = 1 << 19 + // Traverse the powers of two + for mul := 1; mul < 8; mul <<= 1 { + t.Run(strconv.Itoa(mul), func(b *testing.T) { + + ctx := context.Background() + + u := NewUniverse(&NullFetchProtocol{}, "test") + + const testVal = "testval" + g := u.NewGalaxy("testgalaxy", 1024, GetterFunc(func(_ context.Context, key string, dest Codec) error { + return dest.UnmarshalBinary([]byte(testVal)) + })) + + gmp := runtime.GOMAXPROCS(-1) + + grs := gmp * mul + + iters := N / grs + + wg := sync.WaitGroup{} + + for gr := 0; gr < grs; gr++ { + wg.Add(1) + go func() { + defer wg.Done() + cd := ByteCodec{} + for z := 0; z < iters; z++ { + k := "zzzz" + strconv.Itoa(z&0x1fff) + + g.Get(ctx, k, &cd) + } + }() + } + wg.Wait() + + }) + } +} + func BenchmarkGetsParallelManyKeysWithGoroutines(b *testing.B) { // Traverse the powers of two for mul := 1; mul < 128; mul <<= 1 { From 76c2e6fb0a630f8c38e26a3540a6e7fd5a12f458 Mon Sep 17 00:00:00 2001 From: David Finkel Date: Tue, 2 Aug 2022 16:38:44 -0400 Subject: [PATCH 7/7] github actions: run with a released go 1.19 --- .github/workflows/go.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index c9ad9587..bbee5e51 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.0-rc.2'] + goversion: [1.17, 1.18, '1.19'] steps: - name: Set up Go ${{matrix.goversion}} on ${{matrix.os}} uses: actions/setup-go@v3