From f538629c2949e703aae090e530be14c623c92c50 Mon Sep 17 00:00:00 2001 From: Seebs Date: Wed, 15 Sep 2021 13:40:07 -0500 Subject: [PATCH 1/2] record stats for intersectionCallback under the right name --- roaring/roaring.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index 07751d18e..f633c3c1d 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -4520,7 +4520,7 @@ func intersectionCallbackArrayRun(a, b *Container, fn func(uint16)) { } func intersectionCallbackRunRun(a, b *Container, fn func(uint16)) { - statsHit("intersectionCount/RunRun") + statsHit("intersectionCallback/RunRun") ra, rb := a.runs(), b.runs() na, nb := len(ra), len(rb) for i, j := 0, 0; i < na && j < nb; { @@ -4560,14 +4560,14 @@ func intersectionCallbackRunRun(a, b *Container, fn func(uint16)) { } func intersectionCallbackBitmapRun(a, b *Container, fn func(uint16)) { - statsHit("intersectionCount/BitmapRun") + statsHit("intersectionCallback/BitmapRun") for _, iv := range b.runs() { bitmapCallbackRange(a.bitmap(), int32(iv.Start), int32(iv.Last)+1, fn) } } func intersectionCallbackArrayBitmap(a, b *Container, fn func(uint16)) { - statsHit("intersectionCount/ArrayBitmap") + statsHit("intersectionCallback/ArrayBitmap") bitmap := b.bitmap() ln := len(bitmap) for _, val := range a.array() { @@ -4583,7 +4583,7 @@ func intersectionCallbackArrayBitmap(a, b *Container, fn func(uint16)) { } func intersectionCallbackBitmapBitmap(a, b *Container, fn func(uint16)) { - statsHit("intersectionCount/BitmapBitmap") + statsHit("intersectionCallback/BitmapBitmap") ab, bb := a.bitmap(), b.bitmap() for i := range ab { w := ab[i] & bb[i] From 75701d1f71590b890eff1d29996c18a8b5484a62 Mon Sep 17 00:00:00 2001 From: Seebs Date: Wed, 15 Sep 2021 12:36:46 -0500 Subject: [PATCH 2/2] test intersectionCallback more carefully This takes our reasonably broad selection of predefined container types and tries intersectionCallback on each pair of them, comparing results against the results of plain old intersect(). We've had several intersectionCallback fixes recently; every one of them produces test failures here if reverted or broken, so I have at least some confidence in this coverage. Similarly, test everything on containerCallback, verifying that we get the same set of values called back that we get from Slice(). Both of these were verified with -coverprofile to actually be hitting all the lines of code that aren't insane edge case checks like "what if a run is in the wrong order". --- roaring/roaring_internal_test.go | 104 ++++++++++++++++++++++++++++--- 1 file changed, 96 insertions(+), 8 deletions(-) diff --git a/roaring/roaring_internal_test.go b/roaring/roaring_internal_test.go index ecc2b6d97..e041708fb 100644 --- a/roaring/roaring_internal_test.go +++ b/roaring/roaring_internal_test.go @@ -4637,14 +4637,102 @@ func TestContainer_unionInPlace_ArrayUnionRun(t *testing.T) { } } -func TestIntersectionCallback(t *testing.T) { - var hits []uint16 - cb := func(u uint16) { - hits = append(hits, u) +func TestContainerCallback(t *testing.T) { + containers, err := InitContainerArchetypes() + if err != nil { + t.Fatalf("creating containers: %v", err) } - bm := []uint64{0, 5, 0} - bitmapCallbackRange(bm, 64, 69, cb) - if len(hits) != 2 || hits[0] != 64 || hits[1] != 66 { - t.Fatalf("expected 64, 66, got %d", hits) + got := make([]uint16, 65536) + hit := func(u uint16) { + got = append(got, u) + } + var expected []uint16 + // complain() wraps up some pretty-printing logic for this, + // but note also the closure trapping expected/got so we can + // just refer to them without passing them in. + complain := func(t *testing.T, msg string, args ...interface{}) { + l1 := len(expected) + l2 := len(got) + dotdot1 := "" + dotdot2 := "" + if l1 > 8 { + expected = expected[:8] + dotdot1 = "..." + } + if l2 > 8 { + got = got[:8] + dotdot2 = "..." + } + t.Fatalf("%s: expected %d%s, got %d%s", fmt.Sprintf(msg, args...), expected, dotdot1, got, dotdot2) + } + for t1, ci := range containers { + t.Run(ContainerArchetypeNames[t1], func(t *testing.T) { + for _, c1 := range ci { + got = got[:0] + expected = c1.Slice() + containerCallback(c1, hit) + if len(got) != len(expected) { + complain(t, "wrong length (%d vs %d)", len(expected), len(got)) + } + for i := range got { + if got[i] != expected[i] { + complain(t, "element %d differs: expected %d, got %d", i, expected[i], got[i]) + } + } + } + }) + } +} + +func TestIntersectionCallback(t *testing.T) { + containers, err := InitContainerArchetypes() + if err != nil { + t.Fatalf("creating containers: %v", err) + } + got := make([]uint16, 65536) + hit := func(u uint16) { + got = append(got, u) + } + var expected []uint16 + // complain() wraps up some pretty-printing logic for this, + // but note also the closure trapping expected/got so we can + // just refer to them without passing them in. + complain := func(t *testing.T, msg string, args ...interface{}) { + l1 := len(expected) + l2 := len(got) + dotdot1 := "" + dotdot2 := "" + if l1 > 8 { + expected = expected[:8] + dotdot1 = "..." + } + if l2 > 8 { + got = got[:8] + dotdot2 = "..." + } + t.Fatalf("%s: expected %d%s, got %d%s", fmt.Sprintf(msg, args...), expected, dotdot1, got, dotdot2) + } + for t1, ci := range containers { + for t2, cj := range containers { + t.Run(fmt.Sprintf("%s-%s", ContainerArchetypeNames[t1], ContainerArchetypeNames[t2]), func(t *testing.T) { + for _, c1 := range ci { + for _, c2 := range cj { + got = got[:0] + expectedContainer := intersect(c1, c2) + expected = expectedContainer.Slice() + intersectionCallback(c1, c2, hit) + if len(got) != len(expected) { + complain(t, "wrong length (%d vs %d)", len(expected), len(got)) + } + for i := range got { + if got[i] != expected[i] { + complain(t, "element %d differs: expected %d, got %d", i, expected[i], got[i]) + } + } + } + } + + }) + } } }