From 7320ff67ed06883afb7f0c431da8af2b5a8cdb4c Mon Sep 17 00:00:00 2001 From: Vsevolod Strukchinsky Date: Wed, 11 Nov 2026 10:20:39 +0500 Subject: [PATCH 1/2] Make two tests fit 32-bit platforms The first CI run failed in the 32-bit job, for two reasons that have nothing to do with the caches, whose own tests pass there. The inlining check expects what the compiler inlines on amd64 and arm64. On 386 an atomic operation is a call to a few lines of assembly, so the check cannot hold; it now skips on other architectures. TestIndexSize multiplied the largest index size by two in a uint, which overflows 32 bits. The function it tests was right. Co-Authored-By: Claude --- internal/inlinecheck/inline_test.go | 11 +++++++++++ internal/slab/slab_test.go | 5 +++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/internal/inlinecheck/inline_test.go b/internal/inlinecheck/inline_test.go index 1447956..f85e37e 100644 --- a/internal/inlinecheck/inline_test.go +++ b/internal/inlinecheck/inline_test.go @@ -19,6 +19,7 @@ import ( "os" "os/exec" "regexp" + "runtime" "strings" "testing" ) @@ -39,6 +40,16 @@ func TestHotPathsAreInlined(t *testing.T) { if testing.Short() { t.Skip("compiles a package") } + // What the compiler can inline depends on the platform. On these two + // an atomic operation is an instruction or two. On others, 386 among + // them, it is a call to a few lines of assembly whatever this module + // does, and helpers that are made of such calls are too large to be + // inlined in turn. + switch runtime.GOARCH { + case "amd64", "arm64": + default: + t.Skipf("what is inlined on %s is not what this test expects", runtime.GOARCH) + } goTool, err := exec.LookPath("go") if err != nil { t.Skip("there is no go command to compile with") diff --git a/internal/slab/slab_test.go b/internal/slab/slab_test.go index 13d7d3e..bc78760 100644 --- a/internal/slab/slab_test.go +++ b/internal/slab/slab_test.go @@ -132,8 +132,9 @@ func TestIndexSize(t *testing.T) { t.Errorf("indexSize(%d) = %d, want %d", tt.capacity, got, tt.want) } // An empty slot is what ends a probe for a missing key, and the - // load factor stays at or below 2/3. - if 2*got < 3*uint(tt.capacity) || got <= uint(tt.capacity) { + // load factor stays at or below 2/3. Twice the largest index is + // more than 32 bits hold. + if 2*uint64(got) < 3*uint64(tt.capacity) || got <= uint(tt.capacity) { t.Errorf("indexSize(%d) = %d is too small", tt.capacity, got) } } From f4ea4bed1463047a1766a590670784b1c0621e79 Mon Sep 17 00:00:00 2001 From: Vsevolod Strukchinsky Date: Wed, 11 Nov 2026 10:34:43 +0500 Subject: [PATCH 2/2] Give the collector more than one chance in the tests The 32-bit job then failed in another place: a test that counts the keys a cache keeps alive found 32 where 16 are allowed, and 24 after Clear, once, in s3fifo, after it had passed in the run before. No reader touches the cache in that test, so the cache has no reason to hold on to anything, and Clear does not return while it does. What is left is the collector: of a goroutine that it stops in the middle of something it searches the topmost frame for anything that looks like a pointer, and stale stack memory may hold pointers to what a test has dropped. That is transient, where a cache that fails to let go is not. The tests that assert that values are gone now share a helper that runs the collector again while more of them are alive than may be. Co-Authored-By: Claude --- internal/cachetest/cachetest.go | 57 +++++++++++++++++++++++---------- s3fifo/s3fifo_test.go | 22 +++---------- sieve/sieve_test.go | 22 +++---------- 3 files changed, 50 insertions(+), 51 deletions(-) diff --git a/internal/cachetest/cachetest.go b/internal/cachetest/cachetest.go index ee93f13..7ef3ebb 100644 --- a/internal/cachetest/cachetest.go +++ b/internal/cachetest/cachetest.go @@ -13,6 +13,7 @@ import ( "strconv" "sync" "testing" + "time" "weak" "github.com/floatdrop/evict" @@ -26,6 +27,42 @@ type Factory[K comparable, V any] func(capacity, shards int) evict.Cache[K, V] // track of it on its own, which the tests for letting go of values rely on. type Blob [32]byte +// Alive returns how many of the blobs are still alive after the garbage +// collector has had its chance, and gives the collector more chances while +// there are more than limit of them. +// +// What a cache holds on to stays alive however often the collector runs, so a +// cache that fails to let go is found out all the same. But the collector is +// not exact about stacks: of a goroutine that it stops in the middle of +// something, it searches the topmost frame for anything that looks like a +// pointer, and that may be what an earlier use of the memory has left behind. +// A blob that is alive for that reason is gone a few collections later, when +// the goroutine has moved on. The tests of this module have come across it on +// a 32-bit platform, where everything that a function is handed goes through +// the stack. +func Alive(refs []weak.Pointer[Blob], limit int) int { + alive := 0 + for try := range 20 { + if try > 0 { + time.Sleep(time.Millisecond) + } + // The first collection finds the blobs unreachable; it takes + // another one to be sure that the weak pointers know. + runtime.GC() + runtime.GC() + alive = 0 + for _, ref := range refs { + if ref.Value() != nil { + alive++ + } + } + if alive <= limit { + break + } + } + return alive +} + // Factories make the caches that the suite needs. type Factories struct { Int Factory[int, int] @@ -650,20 +687,6 @@ func testKeyIdentity(t *testing.T, newCache Factory[float64, int]) { func letsGo[K comparable, V any](t *testing.T, newCache Factory[K, V], ghosts float64, entry func(i int) (K, V, *Blob)) { t.Helper() - alive := func(refs []weak.Pointer[Blob]) int { - // The first collection finds the blobs unreachable; it takes - // another one to be sure that the weak pointers know. - runtime.GC() - runtime.GC() - n := 0 - for _, ref := range refs { - if ref.Value() != nil { - n++ - } - } - return n - } - for _, tt := range []struct{ capacity, shards int }{{1, 1}, {64, 1}, {1000, 4}, {4096, 0}} { c := newCache(tt.capacity, tt.shards) // A cache may be slow to let go of what it has dropped, but only @@ -682,7 +705,7 @@ func letsGo[K comparable, V any](t *testing.T, newCache Factory[K, V], ghosts fl c.Delete(key) } } - if got, limit := alive(refs), c.Len()+slack; got > limit { + if got, limit := Alive(refs, c.Len()+slack), c.Len()+slack; got > limit { t.Errorf("capacity %d, %d shards: %d blobs are alive with %d entries in the cache, want at most %d", tt.capacity, tt.shards, got, c.Len(), limit) } @@ -698,7 +721,7 @@ func letsGo[K comparable, V any](t *testing.T, newCache Factory[K, V], ghosts fl c.Delete(key) } clear(keys) - if got := alive(refs); c.Len() != 0 || got > slack { + if got := Alive(refs, slack); c.Len() != 0 || got > slack { t.Errorf("capacity %d, %d shards: %d blobs are alive and Len() = %d after every entry was deleted, want at most %d and 0", tt.capacity, tt.shards, got, c.Len(), slack) } @@ -709,7 +732,7 @@ func letsGo[K comparable, V any](t *testing.T, newCache Factory[K, V], ghosts fl c.Set(key, value) } c.Clear() - if got := alive(refs); got != 0 { + if got := Alive(refs, 0); got != 0 { t.Errorf("capacity %d, %d shards: %d blobs are alive after Clear", tt.capacity, tt.shards, got) } diff --git a/s3fifo/s3fifo_test.go b/s3fifo/s3fifo_test.go index a42a24d..98e8e6c 100644 --- a/s3fifo/s3fifo_test.go +++ b/s3fifo/s3fifo_test.go @@ -579,9 +579,7 @@ func TestHeldUpReader(t *testing.T) { case <-time.After(10 * time.Second): t.Fatalf("%d shards: Clear is still waiting although the reader has left", shards) } - runtime.GC() - runtime.GC() - if ref.Value() != nil { + if cachetest.Alive([]weak.Pointer[cachetest.Blob]{ref}, 0) != 0 { t.Errorf("%d shards: the value is alive after Clear", shards) } @@ -746,23 +744,13 @@ func testReadersHelp(t *testing.T, read func(c *s3fifo.Cache[int, *cachetest.Blo } looking.Wait() - alive := func() int { - runtime.GC() - runtime.GC() - n := 0 - for _, ref := range refs { - if ref.Value() != nil { - n++ - } - } - return n - } + alive := func(limit int) int { return cachetest.Alive(refs, limit) } // Everything is deleted. The shard lets go of what it can. for key := range capacity { c.Delete(key) } - if got := alive(); got < held || got > held+2 { + if got := alive(held + 2); got < held || got > held+2 { t.Fatalf("%d values are alive after all were deleted, want the %d that readers look at and up to 2 more", got, held) } @@ -783,7 +771,7 @@ func testReadersHelp(t *testing.T, read func(c *s3fifo.Cache[int, *cachetest.Blo }) } wg.Wait() - if got := alive(); got < held { + if got := alive(len(refs)); got < held { t.Fatalf("%d values are alive, fewer than the %d that readers are looking at", got, held) } @@ -793,7 +781,7 @@ func testReadersHelp(t *testing.T, read func(c *s3fifo.Cache[int, *cachetest.Blo for i := range 400_000 { read(c, i%capacity) } - if got := alive(); got != 0 { + if got := alive(0); got != 0 { t.Errorf("%d values are alive after plenty of reads, want none: the readers did not help", got) } runtime.KeepAlive(c) diff --git a/sieve/sieve_test.go b/sieve/sieve_test.go index f5da5c5..3767d3c 100644 --- a/sieve/sieve_test.go +++ b/sieve/sieve_test.go @@ -381,9 +381,7 @@ func TestHeldUpReader(t *testing.T) { case <-time.After(10 * time.Second): t.Fatalf("%d shards: Clear is still waiting although the reader has left", shards) } - runtime.GC() - runtime.GC() - if ref.Value() != nil { + if cachetest.Alive([]weak.Pointer[cachetest.Blob]{ref}, 0) != 0 { t.Errorf("%d shards: the value is alive after Clear", shards) } @@ -548,23 +546,13 @@ func testReadersHelp(t *testing.T, read func(c *sieve.Cache[int, *cachetest.Blob } looking.Wait() - alive := func() int { - runtime.GC() - runtime.GC() - n := 0 - for _, ref := range refs { - if ref.Value() != nil { - n++ - } - } - return n - } + alive := func(limit int) int { return cachetest.Alive(refs, limit) } // Everything is deleted. The shard lets go of what it can. for key := range capacity { c.Delete(key) } - if got := alive(); got < held || got > held+2 { + if got := alive(held + 2); got < held || got > held+2 { t.Fatalf("%d values are alive after all were deleted, want the %d that readers look at and up to 2 more", got, held) } @@ -585,7 +573,7 @@ func testReadersHelp(t *testing.T, read func(c *sieve.Cache[int, *cachetest.Blob }) } wg.Wait() - if got := alive(); got < held { + if got := alive(len(refs)); got < held { t.Fatalf("%d values are alive, fewer than the %d that readers are looking at", got, held) } @@ -595,7 +583,7 @@ func testReadersHelp(t *testing.T, read func(c *sieve.Cache[int, *cachetest.Blob for i := range 400_000 { read(c, i%capacity) } - if got := alive(); got != 0 { + if got := alive(0); got != 0 { t.Errorf("%d values are alive after plenty of reads, want none: the readers did not help", got) } runtime.KeepAlive(c)