From 6dc8cd7b49de230d8f4e28134d4d093fe64d6e8e Mon Sep 17 00:00:00 2001 From: Seebs Date: Mon, 30 Aug 2021 12:07:03 -0500 Subject: [PATCH 1/2] improve mutex import testing When doing the import tests, import all the data sets if there's multiple data sets, and check that we're producing the correct number of results including overwriting previous values, not just that we produce the same number of values that we set, which shouldn't happen if there's any overlap. Also add a specific test that triggers the case I first ran into this for. --- fragment_internal_test.go | 78 ++++++++++++++++++++++++++++----------- 1 file changed, 56 insertions(+), 22 deletions(-) diff --git a/fragment_internal_test.go b/fragment_internal_test.go index 3820d25fa..a7915fcc3 100644 --- a/fragment_internal_test.go +++ b/fragment_internal_test.go @@ -5543,6 +5543,7 @@ func requireMutexSampleData(tb testing.TB) { // a few mutex tests want common largeish pools of mutex data type mutexSampleData struct { name string + rng mutexSampleRange colIDs, rowIDs [3][]uint64 } @@ -5597,7 +5598,7 @@ func newMutexSampleRange(density int8, rows uint8) mutexSampleRange { return mutexSampleRange((int16(density) << 8) | int16(rows)) } -var sampleMutexData = map[mutexSampleRange]*mutexSampleData{} +var sampleMutexData = map[string]*mutexSampleData{} type mutexDensity struct { name string @@ -5658,7 +5659,7 @@ func prepareMutexSampleData(tb testing.TB) { colIDs := make([]uint64, mutexSampleDataSize) rowIDs := make([]uint64, mutexSampleDataSize) - data := &mutexSampleData{name: d.name + "/" + s.name} + data := &mutexSampleData{name: d.name + "/" + s.name, rng: rng} prev := uint64(0) generated := 0 for idx := 0; int(prev) < len(data.colIDs); idx++ { @@ -5682,42 +5683,75 @@ func prepareMutexSampleData(tb testing.TB) { colIDs[idx] = col % ShardWidth rowIDs[idx] = row } - sampleMutexData[rng] = data + sampleMutexData[data.name] = data } } + d := mutexSampleData{ + name: "extra", + rowIDs: [3][]uint64{ + {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}, + {1}, + {2}, + }, + colIDs: [3][]uint64{ + {0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30, 1, 3, 5, 7, 8, 11, 13, 15, 17, 19, 21, 23}, + {29}, + {33}, + }, + rng: 2, + } + for i := 0; i < 16; i++ { + if (i % 16) != 4 { + d.colIDs[0][i] += 65536 + } + } + sampleMutexData[d.name] = &d } var importBatchSizes = []int{40, 80, 240, 2048} func TestImportMutexSampleData(t *testing.T) { requireMutexSampleData(t) - var scratchCols []uint64 - var scratchRows []uint64 - for rng, data := range sampleMutexData { - scratchCols, scratchRows = data.scratchSpace(0, scratchCols, scratchRows) + var scratchCols [3][]uint64 + var scratchRows [3][]uint64 + for _, data := range sampleMutexData { + for i := range scratchRows { + scratchCols[i], scratchRows[i] = data.scratchSpace(i, scratchCols[i], scratchRows[i]) + } + seen := make(map[uint64]struct{}) t.Run(data.name, func(t *testing.T) { batchSize := 16384 f, _, tx := mustOpenMutexFragment(t, "i", "f", viewStandard, 0, "") defer f.Clean(t) // Set import. var err error - for i := 0; i < len(scratchCols); i += batchSize { - max := i + batchSize - if len(scratchCols) < max { - max = len(scratchCols) + + for i := 0; i < len(scratchCols); i++ { + cols := scratchCols[i] + rows := scratchRows[i] + for _, v := range cols { + seen[v] = struct{}{} } - err = f.bulkImport(tx, scratchRows[i:max:max], scratchCols[i:max:max], &ImportOptions{}) - if err != nil { - t.Fatalf("bulk importing ids [%d:%d]: %v", i, max, err) + + for j := 0; j < len(cols); j += batchSize { + max := j + batchSize + if len(cols) < max { + max = len(cols) + } + err = f.bulkImport(tx, rows[j:max:max], cols[j:max:max], &ImportOptions{}) + if err != nil { + t.Fatalf("bulk importing ids [%d/3] [%d:%d]: %v", i+1, j, max, err) + } + } + count := uint64(0) + for k := uint32(0); k < data.rng.rows(); k++ { + c := f.mustRow(tx, uint64(k)).Count() + count += c + } + if int(count) != len(seen) { + t.Fatalf("for %d rows, %d density, import %d/3: expected %d results, got %d", + data.rng.rows(), data.rng.density(), i+1, len(seen), count) } - } - count := uint64(0) - for k := uint32(0); k < rng.rows(); k++ { - count += f.mustRow(tx, uint64(k)).Count() - } - if int(count) != len(data.colIDs[0]) { - t.Fatalf("for %d rows, %d density: expected %d results, got %d", - rng.rows(), rng.density(), len(data.colIDs[0]), count) } }) } From 0701f9b7ddcff7bfb5a9d11bffa0e5fcee62c9cb Mon Sep 17 00:00:00 2001 From: Seebs Date: Mon, 30 Aug 2021 12:09:14 -0500 Subject: [PATCH 2/2] fix broken intersectionCallback functions Two of the intersectionCallback functions were broken. In intersectionCallbackArrayArray, when checking to see whether we can skip ahead 8, we need to check whether that last value is lower than the one we're looking for, not whether the first value is. For intersectionCallbackArrayBitmap, actually implement it at all; it had never gotten modified significantly from the original intersectionCount, so it still counted and returned intersections, but never called the callback at all. --- roaring/roaring.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index 1cd579593..9567b37fb 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -4493,10 +4493,13 @@ func intersectionCallbackArrayArray(a, b *Container, fn func(uint16)) { if (na << 2) < nb { for _, va := range ca { for cb[0] < va { - if len(cb) > 8 && cb[0] < va { + // try to skip ahead a bit faster + for len(cb) > 7 && cb[7] < va { cb = cb[8:] } - cb = cb[1:] + for len(cb) > 0 && cb[0] < va { + cb = cb[1:] + } if len(cb) == 0 { return } @@ -4585,7 +4588,7 @@ func intersectionCallbackBitmapRun(a, b *Container, fn func(uint16)) { } } -func intersectionCallbackArrayBitmap(a, b *Container, fn func(uint16)) (n int32) { +func intersectionCallbackArrayBitmap(a, b *Container, fn func(uint16)) { statsHit("intersectionCount/ArrayBitmap") bitmap := b.bitmap() ln := len(bitmap) @@ -4595,9 +4598,10 @@ func intersectionCallbackArrayBitmap(a, b *Container, fn func(uint16)) (n int32) break } off := val % 64 - n += int32(bitmap[i]>>off) & 1 + if (bitmap[i]>>off) & 1 != 0 { + fn(val) + } } - return n } func intersectionCallbackBitmapBitmap(a, b *Container, fn func(uint16)) {