From a3ef9bf36078ae9d09f0c5bbe2b8243088259c40 Mon Sep 17 00:00:00 2001 From: Perfloop Agent Date: Thu, 16 Jul 2026 20:28:39 +0000 Subject: [PATCH] perf: speed up fresh Bitmap.Or sorted interleaved source-only high-key inserts Signed-off-by: Perfloop Agent --- bitmap_or_bulk_merge_regression_test.go | 188 +++++++++++++++++ ..._or_bulk_merge_tail_adjacent_bench_test.go | 37 ++++ bitmap_or_bulk_merge_test.go | 196 ++++++++++++++++++ roaring.go | 7 + roaringarray.go | 104 ++++++++++ 5 files changed, 532 insertions(+) create mode 100644 bitmap_or_bulk_merge_regression_test.go create mode 100644 bitmap_or_bulk_merge_tail_adjacent_bench_test.go create mode 100644 bitmap_or_bulk_merge_test.go diff --git a/bitmap_or_bulk_merge_regression_test.go b/bitmap_or_bulk_merge_regression_test.go new file mode 100644 index 00000000..afb8f5d8 --- /dev/null +++ b/bitmap_or_bulk_merge_regression_test.go @@ -0,0 +1,188 @@ +package roaring + +import ( + "bytes" + "encoding/binary" + "testing" +) + +func TestBitmapOrBulkMergeCopyOnWriteTailOwnership(t *testing.T) { + fixture := bitmapOrBulkMergeInterleavedFixture(64, true) + left := fixture.lefts[0] + right := fixture.rights[0] + receiver := left.Clone() + receiver.Or(right) + + tailIndex := right.highlowcontainer.size() - 1 + tailKey := right.highlowcontainer.getKeyAtIndex(tailIndex) + receiverTailIndex := receiver.highlowcontainer.getIndex(tailKey) + if receiverTailIndex < 0 { + t.Fatal("receiver is missing the source-only tail container") + } + if !right.highlowcontainer.needsCopyOnWrite(tailIndex) { + t.Fatal("source-only tail container was not marked copy-on-write") + } + if !receiver.highlowcontainer.needsCopyOnWrite(receiverTailIndex) { + t.Fatal("receiver tail container was not marked copy-on-write") + } + if receiver.highlowcontainer.getContainerAtIndex(receiverTailIndex) != right.highlowcontainer.getContainerAtIndex(tailIndex) { + t.Fatal("source-only tail container was not shared") + } + + receiverValue := uint32(tailKey)<<16 | 10 + sourceValue := uint32(tailKey)<<16 | 11 + receiver.Add(receiverValue) + if right.Contains(receiverValue) { + t.Fatal("receiver tail mutation changed the source") + } + right.Add(sourceValue) + if receiver.Contains(sourceValue) { + t.Fatal("source tail mutation changed the receiver") + } + + if err := receiver.Validate(); err != nil { + t.Fatalf("receiver became invalid after tail mutations: %v", err) + } + if err := right.Validate(); err != nil { + t.Fatalf("source became invalid after tail mutations: %v", err) + } +} + +func bitmapOrBulkMergeReadFromKeys(t *testing.T, keys []uint16) *Bitmap { + t.Helper() + + values := make([]uint32, len(keys)) + for index := range values { + values[index] = uint32(index+1)<<16 | 1 + } + source := BitmapOf(values...) + serialized, err := source.ToBytes() + if err != nil { + t.Fatalf("serialize source: %v", err) + } + if binary.LittleEndian.Uint32(serialized[:4]) != serialCookieNoRunContainer { + t.Fatal("unexpected serialized source layout") + } + for index, key := range keys { + binary.LittleEndian.PutUint16(serialized[8+index*4:], key) + } + + decoded := NewBitmap() + if _, err := decoded.ReadFrom(bytes.NewReader(serialized)); err != nil { + t.Fatalf("deserialize keys: %v", err) + } + if err := decoded.Validate(); err != ErrKeySortOrder { + t.Fatalf("unexpected validation error: %v", err) + } + return decoded +} + +func bitmapOrBulkMergeForwardOr(receiver, source *Bitmap) { + pos1 := 0 + pos2 := 0 + length1 := receiver.highlowcontainer.size() + length2 := source.highlowcontainer.size() +main: + for (pos1 < length1) && (pos2 < length2) { + s1 := receiver.highlowcontainer.getKeyAtIndex(pos1) + s2 := source.highlowcontainer.getKeyAtIndex(pos2) + + for { + if s1 < s2 { + pos1++ + if pos1 == length1 { + break main + } + s1 = receiver.highlowcontainer.getKeyAtIndex(pos1) + } else if s1 > s2 { + receiver.highlowcontainer.insertNewKeyValueAt(pos1, s2, source.highlowcontainer.getContainerAtIndex(pos2).clone()) + pos1++ + length1++ + pos2++ + if pos2 == length2 { + break main + } + s2 = source.highlowcontainer.getKeyAtIndex(pos2) + } else { + newContainer := receiver.highlowcontainer.getUnionedWritableContainer(pos1, source.highlowcontainer.getContainerAtIndex(pos2)) + receiver.highlowcontainer.replaceKeyAndContainerAtIndex(pos1, s1, newContainer, false) + pos1++ + pos2++ + if pos1 == length1 || pos2 == length2 { + break main + } + s1 = receiver.highlowcontainer.getKeyAtIndex(pos1) + s2 = source.highlowcontainer.getKeyAtIndex(pos2) + } + } + } + if pos1 == length1 { + receiver.highlowcontainer.appendCopyMany(source.highlowcontainer, pos2, length2) + } +} + +func assertBitmapOrBulkMergeState(t *testing.T, got, want *Bitmap) { + t.Helper() + + if !got.highlowcontainer.equals(want.highlowcontainer) { + t.Fatalf("metadata differs: got keys %v, want %v", got.highlowcontainer.keys, want.highlowcontainer.keys) + } + if got.highlowcontainer.copyOnWrite != want.highlowcontainer.copyOnWrite { + t.Fatalf("copy-on-write state differs: got %t, want %t", got.highlowcontainer.copyOnWrite, want.highlowcontainer.copyOnWrite) + } + if len(got.highlowcontainer.needCopyOnWrite) != len(want.highlowcontainer.needCopyOnWrite) { + t.Fatalf("copy-on-write marker length differs: got %d, want %d", len(got.highlowcontainer.needCopyOnWrite), len(want.highlowcontainer.needCopyOnWrite)) + } + for index, gotMarker := range got.highlowcontainer.needCopyOnWrite { + if wantMarker := want.highlowcontainer.needCopyOnWrite[index]; gotMarker != wantMarker { + t.Fatalf("copy-on-write marker %d differs: got %t, want %t", index, gotMarker, wantMarker) + } + } + if gotErr, wantErr := got.Validate(), want.Validate(); gotErr != wantErr { + t.Fatalf("validation differs: got %v, want %v", gotErr, wantErr) + } +} + +func TestBitmapOrBulkMergeFallsBackForUnsortedKeys(t *testing.T) { + tests := []struct { + name string + receiver *Bitmap + source *Bitmap + }{ + { + name: "source", + receiver: BitmapOf(uint32(2)<<16|2, uint32(4)<<16|2), + source: bitmapOrBulkMergeReadFromKeys(t, []uint16{1, 2, 4, 1}), + }, + { + name: "receiver", + receiver: bitmapOrBulkMergeReadFromKeys(t, []uint16{2, 4, 2}), + source: BitmapOf( + uint32(1)<<16|2, + uint32(2)<<16|2, + uint32(4)<<16|2, + uint32(5)<<16|2, + ), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + probe := test.receiver.Clone() + if probe.highlowcontainer.orBulk(&test.source.highlowcontainer, 0, 0) { + t.Fatal("bulk merge accepted unsorted metadata") + } + + want := test.receiver.Clone() + bitmapOrBulkMergeForwardOr(want, test.source) + got := test.receiver.Clone() + defer func() { + if recovered := recover(); recovered != nil { + t.Fatalf("Bitmap.Or panicked: %v", recovered) + } + }() + got.Or(test.source) + assertBitmapOrBulkMergeState(t, got, want) + }) + } +} diff --git a/bitmap_or_bulk_merge_tail_adjacent_bench_test.go b/bitmap_or_bulk_merge_tail_adjacent_bench_test.go new file mode 100644 index 00000000..c752ee6b --- /dev/null +++ b/bitmap_or_bulk_merge_tail_adjacent_bench_test.go @@ -0,0 +1,37 @@ +package roaring + +import "testing" + +func bitmapOrBulkMergeTailAdjacentFixture() bitmapOrBulkMergeFixture { + const containers = 4096 + + leftKeys := make([]uint16, 0, containers-1) + for key := 0; key < containers-2; key++ { + leftKeys = append(leftKeys, uint16(key)) + } + leftKeys = append(leftKeys, containers-1) + + return newBitmapOrBulkMergeFixture(leftKeys, []uint16{containers - 2}, false) +} + +func BenchmarkBitmapOrBulkMergeTailAdjacent(b *testing.B) { + b.Run("fresh-single-interior-4096", func(b *testing.B) { + b.Run("fresh-tail-adjacent-4096", func(b *testing.B) { + fixture := bitmapOrBulkMergeTailAdjacentFixture() + + b.ReportAllocs() + var cardinality uint64 + b.ResetTimer() + for i := 0; i < b.N; i++ { + fixtureIndex := i & 1 + receiver := fixture.lefts[fixtureIndex].Clone() + receiver.Or(fixture.rights[fixtureIndex]) + cardinality += receiver.GetCardinality() + } + b.StopTimer() + if cardinality != fixture.cardinality*uint64(b.N) { + b.Fatalf("unexpected total cardinality: got %d, want %d", cardinality, fixture.cardinality*uint64(b.N)) + } + }) + }) +} diff --git a/bitmap_or_bulk_merge_test.go b/bitmap_or_bulk_merge_test.go new file mode 100644 index 00000000..2c7fe590 --- /dev/null +++ b/bitmap_or_bulk_merge_test.go @@ -0,0 +1,196 @@ +package roaring + +import "testing" + +type bitmapOrBulkMergeFixture struct { + lefts [2]*Bitmap + rights [2]*Bitmap + cardinality uint64 +} + +func newBitmapOrBulkMergeFixture(leftKeys, rightKeys []uint16, copyOnWrite bool) bitmapOrBulkMergeFixture { + fixture := bitmapOrBulkMergeFixture{} + for variant := range fixture.lefts { + left := NewBitmap() + right := NewBitmap() + leftLow := uint16(variant * 2) + rightLow := leftLow + 1 + for _, key := range leftKeys { + left.Add(uint32(key)<<16 | uint32(leftLow)) + } + for _, key := range rightKeys { + right.Add(uint32(key)<<16 | uint32(rightLow)) + } + if copyOnWrite { + left.SetCopyOnWrite(true) + right.SetCopyOnWrite(true) + } + fixture.lefts[variant] = left + fixture.rights[variant] = right + } + fixture.cardinality = bitmapOrBulkMergeExpected(fixture.lefts[0], fixture.rights[0]).GetCardinality() + return fixture +} + +func bitmapOrBulkMergeExpected(left, right *Bitmap) *Bitmap { + values := make([]uint32, 0, left.GetCardinality()+right.GetCardinality()) + values = append(values, left.ToArray()...) + values = append(values, right.ToArray()...) + return BitmapOf(values...) +} + +func bitmapOrBulkMergeKeys(start, count, step int) []uint16 { + keys := make([]uint16, count) + for i := range keys { + keys[i] = uint16(start + i*step) + } + return keys +} + +func bitmapOrBulkMergeInterleavedFixture(containers int, copyOnWrite bool) bitmapOrBulkMergeFixture { + return newBitmapOrBulkMergeFixture( + bitmapOrBulkMergeKeys(0, containers, 2), + bitmapOrBulkMergeKeys(1, containers, 2), + copyOnWrite, + ) +} + +func bitmapOrBulkMergeAppendFixture(containers int) bitmapOrBulkMergeFixture { + return newBitmapOrBulkMergeFixture( + bitmapOrBulkMergeKeys(0, containers, 1), + bitmapOrBulkMergeKeys(containers, containers, 1), + false, + ) +} + +func bitmapOrBulkMergeOverlapFixture(containers int) bitmapOrBulkMergeFixture { + keys := bitmapOrBulkMergeKeys(0, containers, 1) + return newBitmapOrBulkMergeFixture(keys, keys, false) +} + +func bitmapOrBulkMergeSingleInteriorFixture(containers int) bitmapOrBulkMergeFixture { + leftKeys := make([]uint16, 0, containers-1) + middle := containers / 2 + for key := 0; key < containers; key++ { + if key != middle { + leftKeys = append(leftKeys, uint16(key)) + } + } + return newBitmapOrBulkMergeFixture(leftKeys, []uint16{uint16(middle)}, false) +} + +func bitmapOrBulkMergeFixtureCases() map[string]bitmapOrBulkMergeFixture { + return map[string]bitmapOrBulkMergeFixture{ + "interleaved-64": bitmapOrBulkMergeInterleavedFixture(64, false), + "interleaved-65": bitmapOrBulkMergeInterleavedFixture(65, false), + "interleaved-1024": bitmapOrBulkMergeInterleavedFixture(1024, false), + "interleaved-4096": bitmapOrBulkMergeInterleavedFixture(4096, false), + "append-only-4096": bitmapOrBulkMergeAppendFixture(4096), + "overlap-4096": bitmapOrBulkMergeOverlapFixture(4096), + "copy-on-write-interleaved-4096": bitmapOrBulkMergeInterleavedFixture(4096, true), + "single-interior-4096": bitmapOrBulkMergeSingleInteriorFixture(4096), + "mixed-key-order": newBitmapOrBulkMergeFixture( + []uint16{2, 4, 6, 8}, + []uint16{1, 4, 5, 8, 9}, + false, + ), + } +} + +func TestBitmapOrBulkMergeFixtures(t *testing.T) { + for name, fixture := range bitmapOrBulkMergeFixtureCases() { + t.Run(name, func(t *testing.T) { + left := fixture.lefts[0] + right := fixture.rights[0] + want := bitmapOrBulkMergeExpected(left, right) + receiver := left.Clone() + receiver.Or(right) + + if !receiver.Equals(want) { + t.Fatalf("unexpected union: got %v, want %v", receiver, want) + } + if receiver.GetCardinality() != fixture.cardinality { + t.Fatalf("unexpected cardinality: got %d, want %d", receiver.GetCardinality(), fixture.cardinality) + } + if err := receiver.Validate(); err != nil { + t.Fatalf("union produced an invalid bitmap: %v", err) + } + }) + } +} + +func TestBitmapOrBulkMergeCopyOnWriteOwnership(t *testing.T) { + fixture := bitmapOrBulkMergeInterleavedFixture(64, true) + left := fixture.lefts[0] + right := fixture.rights[0] + receiver := left.Clone() + receiver.Or(right) + + const ( + leftKey = uint32(0) << 16 + rightKey = uint32(1) << 16 + receiver1 = uint32(10) + receiver2 = uint32(11) + ) + + receiver.Add(rightKey | receiver1) + if right.Contains(rightKey | receiver1) { + t.Fatal("receiver mutation changed a source-only container") + } + right.Add(rightKey | receiver2) + if receiver.Contains(rightKey | receiver2) { + t.Fatal("source mutation changed a receiver source-only container") + } + + receiver.Add(leftKey | receiver1) + if left.Contains(leftKey | receiver1) { + t.Fatal("receiver mutation changed a receiver-only container") + } + left.Add(leftKey | receiver2) + if receiver.Contains(leftKey | receiver2) { + t.Fatal("left mutation changed a receiver container") + } + + if err := receiver.Validate(); err != nil { + t.Fatalf("receiver became invalid after copy-on-write mutations: %v", err) + } + if err := left.Validate(); err != nil { + t.Fatalf("left became invalid after copy-on-write mutations: %v", err) + } + if err := right.Validate(); err != nil { + t.Fatalf("right became invalid after copy-on-write mutations: %v", err) + } +} + +func BenchmarkBitmapOrBulkMerge(b *testing.B) { + fixtures := []struct { + name string + fixture bitmapOrBulkMergeFixture + }{ + {"fresh-interleaved-64", bitmapOrBulkMergeInterleavedFixture(64, false)}, + {"fresh-interleaved-65", bitmapOrBulkMergeInterleavedFixture(65, false)}, + {"fresh-interleaved-1024", bitmapOrBulkMergeInterleavedFixture(1024, false)}, + {"fresh-interleaved-4096", bitmapOrBulkMergeInterleavedFixture(4096, false)}, + {"fresh-append-only-4096", bitmapOrBulkMergeAppendFixture(4096)}, + {"fresh-overlap-4096", bitmapOrBulkMergeOverlapFixture(4096)}, + {"fresh-copy-on-write-interleaved-4096", bitmapOrBulkMergeInterleavedFixture(4096, true)}, + {"fresh-single-interior-4096", bitmapOrBulkMergeSingleInteriorFixture(4096)}, + } + for _, benchmark := range fixtures { + b.Run(benchmark.name, func(b *testing.B) { + b.ReportAllocs() + var cardinality uint64 + b.ResetTimer() + for i := 0; i < b.N; i++ { + fixtureIndex := i & 1 + receiver := benchmark.fixture.lefts[fixtureIndex].Clone() + receiver.Or(benchmark.fixture.rights[fixtureIndex]) + cardinality += receiver.GetCardinality() + } + b.StopTimer() + if cardinality != benchmark.fixture.cardinality*uint64(b.N) { + b.Fatalf("unexpected total cardinality: got %d, want %d", cardinality, benchmark.fixture.cardinality*uint64(b.N)) + } + }) + } +} diff --git a/roaring.go b/roaring.go index cf6bcdaa..f1458d32 100644 --- a/roaring.go +++ b/roaring.go @@ -1632,6 +1632,7 @@ func (rb *Bitmap) Or(x2 *Bitmap) { pos2 := 0 length1 := rb.highlowcontainer.size() length2 := x2.highlowcontainer.size() + bulkMergeAvailable := true main: for (pos1 < length1) && (pos2 < length2) { s1 := rb.highlowcontainer.getKeyAtIndex(pos1) @@ -1645,6 +1646,12 @@ main: } s1 = rb.highlowcontainer.getKeyAtIndex(pos1) } else if s1 > s2 { + if bulkMergeAvailable { + if rb.highlowcontainer.orBulk(&x2.highlowcontainer, pos1, pos2) { + return + } + bulkMergeAvailable = false + } rb.highlowcontainer.insertNewKeyValueAt(pos1, s2, x2.highlowcontainer.getContainerAtIndex(pos2).clone()) pos1++ length1++ diff --git a/roaringarray.go b/roaringarray.go index a1a71784..8548c3a3 100644 --- a/roaringarray.go +++ b/roaringarray.go @@ -384,6 +384,110 @@ func (ra *roaringArray) insertNewKeyValueAt(i int, key uint16, value container) ra.needCopyOnWrite[i] = false } +func (ra *roaringArray) growTo(newsize int) { + if cap(ra.keys) < newsize { + keys := make([]uint16, newsize) + copy(keys, ra.keys) + ra.keys = keys + } else { + ra.keys = ra.keys[:newsize] + } + if cap(ra.containers) < newsize { + containers := make([]container, newsize) + copy(containers, ra.containers) + ra.containers = containers + } else { + ra.containers = ra.containers[:newsize] + } + if cap(ra.needCopyOnWrite) < newsize { + needCopyOnWrite := make([]bool, newsize) + copy(needCopyOnWrite, ra.needCopyOnWrite) + ra.needCopyOnWrite = needCopyOnWrite + } else { + ra.needCopyOnWrite = ra.needCopyOnWrite[:newsize] + } +} + +func (ra *roaringArray) copyOrSourceContainerAt(other *roaringArray, index int, receiverLastKey uint16) (container, bool) { + if other.keys[index] > receiverLastKey { + copyOnWrite := (ra.copyOnWrite && other.copyOnWrite) || other.needsCopyOnWrite(index) + if copyOnWrite { + if !other.needsCopyOnWrite(index) { + other.setNeedsCopyOnWrite(index) + } + return other.containers[index], true + } + } + return other.containers[index].clone(), false +} + +func (ra *roaringArray) orBulk(other *roaringArray, pos1, pos2 int) bool { + if !ra.checkKeysSorted() || !other.checkKeysSorted() { + return false + } + + length1 := ra.size() + length2 := other.size() + receiverLastKey := ra.getKeyAtIndex(length1 - 1) + sourceOnly := 0 + left, right := pos1, pos2 + for left < length1 && right < length2 { + leftKey := ra.getKeyAtIndex(left) + rightKey := other.getKeyAtIndex(right) + if leftKey < rightKey { + left++ + } else if leftKey > rightKey { + sourceOnly++ + right++ + } else { + left++ + right++ + } + } + sourceOnly += length2 - right + + ra.growTo(length1 + sourceOnly) + destination := length1 + sourceOnly - 1 + left = length1 - 1 + right = length2 - 1 + for left >= pos1 && right >= pos2 { + leftKey := ra.getKeyAtIndex(left) + rightKey := other.getKeyAtIndex(right) + if leftKey > rightKey { + leftContainer := ra.getContainerAtIndex(left) + leftNeedsCopyOnWrite := ra.needsCopyOnWrite(left) + ra.replaceKeyAndContainerAtIndex(destination, leftKey, leftContainer, leftNeedsCopyOnWrite) + left-- + } else if leftKey < rightKey { + rightContainer, rightNeedsCopyOnWrite := ra.copyOrSourceContainerAt(other, right, receiverLastKey) + ra.replaceKeyAndContainerAtIndex(destination, rightKey, rightContainer, rightNeedsCopyOnWrite) + right-- + } else { + union := ra.getUnionedWritableContainer(left, other.getContainerAtIndex(right)) + ra.replaceKeyAndContainerAtIndex(destination, leftKey, union, false) + left-- + right-- + } + destination-- + } + for left >= pos1 { + leftKey := ra.getKeyAtIndex(left) + leftContainer := ra.getContainerAtIndex(left) + leftNeedsCopyOnWrite := ra.needsCopyOnWrite(left) + ra.replaceKeyAndContainerAtIndex(destination, leftKey, leftContainer, leftNeedsCopyOnWrite) + left-- + destination-- + } + for right >= pos2 { + rightKey := other.getKeyAtIndex(right) + rightContainer, rightNeedsCopyOnWrite := ra.copyOrSourceContainerAt(other, right, receiverLastKey) + ra.replaceKeyAndContainerAtIndex(destination, rightKey, rightContainer, rightNeedsCopyOnWrite) + right-- + destination-- + } + return true +} + func (ra *roaringArray) remove(key uint16) bool { i := ra.binarySearch(0, int64(len(ra.keys)), key) if i >= 0 { // if a new key