Merge pull request #29 from vimeo/typed_ll_invariant_fix

Fix an invariant violation in the typed Linked List implementation and add testing/benchmarking infrastructure
vimeo · Aug 2, 2022 · 4270c82 · 4270c82
2 parents 0ec222b + 76c2e6f
commit 4270c82
Show file tree

Hide file tree

Showing 5 changed files with 285 additions and 1 deletion.
diff --git 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
@@ -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/galaxycache_test.go b/galaxycache_test.go
@@ -24,6 +24,7 @@ import (
 	"fmt"
 	"math"
 	"math/rand"
+	"runtime"
 	"strconv"
 	"strings"
 	"sync"
@@ -704,3 +705,159 @@ 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)
+		}
+	})
+}
+
+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 {
+		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()
+
+		})
+	}
+}
diff --git 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)
+	}
+}
diff --git 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
@@ -77,9 +96,43 @@ 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]) 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
@@ -82,4 +82,50 @@ 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) {
+	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)
+	}
 }