diff --git a/bench_test.go b/bench_test.go index befdd0d..d19795f 100644 --- a/bench_test.go +++ b/bench_test.go @@ -358,6 +358,10 @@ func createFixedBounds(step int, random *rand.Rand) (forward, reverse []Bounds) func createBenchStoreConfig(corpusName string, entries map[string]byte) *storeConfig { random := rand.New(rand.NewPCG(uint64(len(entries)), 4839028453)) + ref := newReference() + for k, v := range entries { + ref.Set([]byte(k), v) + } present := createPresent(entries, random) absent := createAbsent(entries, len(present)-1, random) keys := append(slices.Concat(present...), slices.Concat(absent...)...) @@ -366,7 +370,7 @@ func createBenchStoreConfig(corpusName string, entries map[string]byte) *storeCo return &storeConfig{ name: fmt.Sprintf("corpus=%s/size=%d", corpusName, len(entries)), size: len(entries), - entries: entries, + ref: ref, present: present, absent: absent, forward: forward, @@ -407,8 +411,8 @@ func BenchmarkCreate(b *testing.B) { b.Run(bench.name, func(b *testing.B) { for b.Loop() { store := bench.def.factory() - for k, v := range bench.config.entries { - store.Set([]byte(k), v) + for k, v := range bench.config.ref.All() { + store.Set(k, v) } } }) diff --git a/fuzz_test.go b/fuzz_test.go index e1afc8b..85826f2 100644 --- a/fuzz_test.go +++ b/fuzz_test.go @@ -54,11 +54,11 @@ func createFuzzStoreConfigs(size int) []*storeConfig { random := rand.New(rand.NewPCG(rand.Uint64(), rand.Uint64())) config.name = "fuzz" config.size = size - config.entries = map[string]byte{} + config.ref = newReference() for count := 0; count < size; { - key := string(randomKey(fuzzMeanKeyLen, random)) - if _, ok := config.entries[key]; !ok { - config.entries[key] = randomByte(random) + key := randomKey(fuzzMeanKeyLen, random) + if _, ok := config.ref.Get(key); !ok { + config.ref.Set(key, randomByte(random)) count++ } } @@ -67,11 +67,10 @@ func createFuzzStoreConfigs(size int) []*storeConfig { func FuzzGet(f *testing.F) { fuzzStores := createTestStores(fuzzStoreConfigs) - ref := createReferenceStore(fuzzStoreConfigs[0]) f.Fuzz(func(t *testing.T, fuzzKey uint32, fuzzKeyLen byte) { key := keyForFuzzInputs(fuzzKey, fuzzKeyLen) - expected, expectedOk := ref.Get(key) for _, fuzz := range fuzzStores { + expected, expectedOk := fuzz.config.ref.Get(key) actual, actualOk := fuzz.store.Get(key) assert.Equal(t, expectedOk, actualOk, "%s: %s", fuzz.def.name, kv.KeyName(key)) assert.Equal(t, expected, actual, "%s: %s", fuzz.def.name, kv.KeyName(key)) @@ -81,7 +80,10 @@ func FuzzGet(f *testing.F) { func FuzzSet(f *testing.F) { fuzzStores := createTestStores(fuzzStoreConfigs) - ref := createReferenceStore(fuzzStoreConfigs[0]) + // This only works because there is only one fuzz store config. + // This is unfortunately necessary because configs are shared between TestStores. + // This needs to be fixed. + ref := fuzzStoreConfigs[0].ref.Clone() f.Fuzz(func(t *testing.T, fuzzKey uint32, fuzzKeyLen, value byte) { key := keyForFuzzInputs(fuzzKey, fuzzKeyLen) expected, expectedOk := ref.Set(key, value) @@ -98,7 +100,10 @@ func FuzzSet(f *testing.F) { func FuzzDelete(f *testing.F) { fuzzStores := createTestStores(fuzzStoreConfigs) - ref := createReferenceStore(fuzzStoreConfigs[0]) + // This only works because there is only one fuzz store config. + // This is unfortunately necessary because configs are shared between TestStores. + // This needs to be fixed. + ref := fuzzStoreConfigs[0].ref.Clone() f.Fuzz(func(t *testing.T, fuzzKey uint32, fuzzKeyLen byte) { key := keyForFuzzInputs(fuzzKey, fuzzKeyLen) expected, expectedOk := ref.Delete(key) @@ -115,7 +120,6 @@ func FuzzDelete(f *testing.F) { func FuzzRange(f *testing.F) { fuzzStores := createTestStores(fuzzRangeStoreConfigs) - ref := createReferenceStore(fuzzRangeStoreConfigs[0]) f.Fuzz(func(t *testing.T, fuzzBeginKey, fuzzEndKey uint32, fuzzBeginKeyLen, fuzzEndKeyLen byte) { begin := keyForFuzzInputs(fuzzBeginKey, fuzzBeginKeyLen) end := keyForFuzzInputs(fuzzEndKey, fuzzEndKeyLen) @@ -127,18 +131,21 @@ func FuzzRange(f *testing.F) { } forward := From(begin).To(end) reverse := From(end).DownTo(begin) - refForward := ref.Range(forward) - refReverse := ref.Range(reverse) for _, fuzz := range fuzzStores { - assertItersEqual(t, refForward, fuzz.store.Range(forward), "%s: %s", fuzz.def.name, forward) - assertItersEqual(t, refReverse, fuzz.store.Range(reverse), "%s: %s", fuzz.def.name, reverse) + assertItersEqual(t, fuzz.config.ref.Range(forward), fuzz.store.Range(forward), + "%s: %s", fuzz.def.name, forward) + assertItersEqual(t, fuzz.config.ref.Range(reverse), fuzz.store.Range(reverse), + "%s: %s", fuzz.def.name, reverse) } }) } func FuzzMixed(f *testing.F) { fuzzStores := createTestStores(fuzzStoreConfigs) - ref := createReferenceStore(fuzzStoreConfigs[0]) + // This only works because there is only one fuzz store config. + // This is unfortunately necessary because configs are shared between TestStores. + // This needs to be fixed. + ref := fuzzStoreConfigs[0].ref.Clone() f.Fuzz(func(t *testing.T, fuzzSetKey, fuzzDeleteKey uint32, fuzzSetKeyLen, fuzzDeleteKeyLen, value byte) { key := keyForFuzzInputs(fuzzSetKey, fuzzSetKeyLen) expected, expectedOk := ref.Set(key, value) diff --git a/kv_test.go b/kv_test.go index e80f296..a55e0a0 100644 --- a/kv_test.go +++ b/kv_test.go @@ -1,11 +1,9 @@ package kv_test import ( - "bytes" "fmt" "iter" "math/bits" - "slices" "strings" "testing" @@ -31,13 +29,8 @@ type ( // shared by all store implementations. storeConfig struct { name string - - // The number of key/value pairs in stores generated by this config. - // Also the size of entries. size int - - // The key/value entries in stores generated by this config. - entries map[string]byte + ref *reference // present/absent[i] = a set of keys of length i that are present/absent. // For denser stores, there may be no absent keys of shorter lengths. @@ -66,7 +59,7 @@ const ( var ( implDefs = []*implDef{ - {"impl=reference", newReference}, + {"impl=reference", func() TestStore { return TestStore(newReference()) }}, {"impl=pointer-trie", asCloneable(kv.NewPointerTrie[byte])}, {"impl=array-trie", asCloneable(kv.NewArrayTrie[byte])}, } @@ -140,6 +133,9 @@ var ( testStoreConfigs = createTestStoreConfigs() ) +// nextKey and prevKey may get promoted to exported functions if there's a good use case. +// Probably only if allowing an in-progress iteration to be advanced (seek). + func nextKey(key []byte) []byte { if key == nil { panic("key must be non-nil") @@ -244,7 +240,7 @@ func createTestStoreConfigs() []*storeConfig { config := storeConfig{ fmt.Sprintf("sub-store=%0*b", len(testPresentKeys), keyBits), bits.OnesCount(uint(keyBits)), - map[string]byte{}, + newReference(), make([]keySet, testMaxKeyLen+1), make([]keySet, testMaxKeyLen+1), forward, @@ -254,7 +250,7 @@ func createTestStoreConfigs() []*storeConfig { for i, k := range testPresentKeys { keyLen := len(k) if keyBits&mask != 0 { - config.entries[string(k)] = byte(i) + config.ref.Set(k, byte(i)) config.present[keyLen] = append(config.present[keyLen], k) } else { config.absent[keyLen] = append(config.absent[keyLen], k) @@ -270,21 +266,13 @@ func createTestStoreConfigs() []*storeConfig { return result } -func createReferenceStore(config *storeConfig) TestStore { - store := newReference() - for k, v := range config.entries { - store.Set([]byte(k), v) - } - return store -} - func createTestStores(storeConfigs []*storeConfig) []*testStore { result := []*testStore{} for _, config := range storeConfigs { for _, def := range implDefs { store := def.factory() - for k, v := range config.entries { - store.Set([]byte(k), v) + for k, v := range config.ref.All() { + store.Set(k, v) } name := config.name + "/" + def.name result = append(result, &testStore{name, store, def, config}) @@ -328,35 +316,16 @@ func assertAbsent(t *testing.T, key []byte, store TestStore) { } } -// Temporary. -func entryIter(entries map[string]byte, keys iter.Seq[[]byte]) iter.Seq2[[]byte, byte] { - return func(yield func([]byte, byte) bool) { - for k := range keys { - v, ok := entries[string(k)] - if !ok { - panic(fmt.Sprintf("key %s not found", kv.KeyName(k))) - } - if !yield(k, v) { - return - } - } - } -} - // Test that store contains only the key/value pairs in entries, // and that Range(forward/reverse) returns them in the correct order. -func assertSame(t *testing.T, entries map[string]byte, store TestStore) { - keys := [][]byte{} - for k, v := range entries { - actual, ok := store.Get([]byte(k)) +func assertSame(t *testing.T, expected *reference, actual TestStore) { + for k, v := range expected.All() { + actual, ok := actual.Get(k) assert.True(t, ok) assert.Equal(t, v, actual) - keys = append(keys, []byte(k)) } - slices.SortFunc(keys, bytes.Compare) - assertItersEqual(t, entryIter(entries, slices.Values(keys)), store.Range(forwardAll)) - slices.Reverse(keys) - assertItersEqual(t, entryIter(entries, slices.Values(keys)), store.Range(reverseAll)) + assertItersEqual(t, expected.Range(forwardAll), actual.Range(forwardAll)) + assertItersEqual(t, expected.Range(reverseAll), actual.Range(reverseAll)) } func TestNilArgPanics(t *testing.T) { @@ -387,33 +356,34 @@ func TestNilArgPanics(t *testing.T) { func testKey(t *testing.T, key []byte, store TestStore) { const value = byte(43) const replacement = byte(57) - existing := map[string]byte{} + ref := newReference() for k, v := range store.Range(forwardAll) { - existing[string(k)] = v + ref.Set(k, v) } - require.NotContains(t, existing, string(key)) + _, ok := ref.Get(key) + require.False(t, ok) assertAbsent(t, key, store) - assertSame(t, existing, store) + assertSame(t, ref, store) + ref.Set(key, value) actual, ok := store.Set(key, value) assert.False(t, ok) assert.Equal(t, zero, actual) - existing[string(key)] = value - assertSame(t, existing, store) + assertSame(t, ref, store) + ref.Set(key, replacement) actual, ok = store.Set(key, replacement) assert.True(t, ok) assert.Equal(t, value, actual) - existing[string(key)] = replacement - assertSame(t, existing, store) + assertSame(t, ref, store) + ref.Delete(key) actual, ok = store.Delete(key) assert.True(t, ok) assert.Equal(t, replacement, actual) assertAbsent(t, key, store) - delete(existing, string(key)) - assertSame(t, existing, store) + assertSame(t, ref, store) } // The empty key is often a special case in an implementation. @@ -558,17 +528,17 @@ func TestStores(t *testing.T) { // Build the store, testing along the way. store := test.def.factory() - existing := map[string]byte{} - for k, v := range test.config.entries { - t.Run("op=set/key="+kv.KeyName([]byte(k)), func(t *testing.T) { - assertAbsent(t, []byte(k), store) - assertSame(t, existing, store) + ref := newReference() + for k, v := range test.config.ref.All() { + t.Run("op=set/key="+kv.KeyName(k), func(t *testing.T) { + assertAbsent(t, k, store) + assertSame(t, ref, store) - actual, ok := store.Set([]byte(k), v) + actual, ok := store.Set(k, v) assert.False(t, ok) assert.Equal(t, zero, actual) - existing[k] = v - assertSame(t, existing, store) + ref.Set(k, v) + assertSame(t, ref, store) }) } @@ -581,12 +551,11 @@ func TestStores(t *testing.T) { } t.Run("op=range", func(t *testing.T) { - ref := createReferenceStore(test.config) for _, bounds := range test.config.forward { - assertItersEqual(t, ref.Range(&bounds), store.Range(&bounds), "%s", &bounds) + assertItersEqual(t, test.config.ref.Range(&bounds), store.Range(&bounds), "%s", &bounds) } for _, bounds := range test.config.reverse { - assertItersEqual(t, ref.Range(&bounds), store.Range(&bounds), "%s", &bounds) + assertItersEqual(t, test.config.ref.Range(&bounds), store.Range(&bounds), "%s", &bounds) } assertEarlyYield(t, test.config.size, store.Range(forwardAll)) assertEarlyYield(t, test.config.size, store.Range(reverseAll)) @@ -602,36 +571,36 @@ func TestClone(t *testing.T) { t.Run(test.name, func(t *testing.T) { t.Parallel() original := test.store - assertSame(t, test.config.entries, original) + assertSame(t, test.config.ref, original) // test that the clone was correct store := original.Clone() - assertSame(t, test.config.entries, store) + assertSame(t, test.config.ref, store) // mutate the clone and test that original hasn't changed - for k := range test.config.entries { - store.Delete([]byte(k)) + for k := range test.config.ref.All() { + store.Delete(k) } - assertSame(t, map[string]byte{}, store) + assertIterEmpty(t, store.Range(forwardAll)) for _, keys := range test.config.absent { for i, k := range keys { store.Set(k, byte(i)) } } - assertSame(t, test.config.entries, original) + assertSame(t, test.config.ref, original) // mutate the original and test that the clone hasn't changed store = original.Clone() - for k := range test.config.entries { - original.Delete([]byte(k)) + for k := range test.config.ref.All() { + original.Delete(k) } - assertSame(t, map[string]byte{}, original) + assertIterEmpty(t, original.Range(forwardAll)) for _, keys := range test.config.absent { for i, k := range keys { original.Set(k, byte(i)) } } - assertSame(t, test.config.entries, store) + assertSame(t, test.config.ref, store) }) } } diff --git a/reference_test.go b/reference_test.go index ad327bb..0ead9de 100644 --- a/reference_test.go +++ b/reference_test.go @@ -20,7 +20,7 @@ type reference struct { dirty bool } -func newReference() TestStore { +func newReference() *reference { return &reference{ entries: map[string]byte{}, }