From e4bdc6db7e20c3a3d7125e51d1d50542fcd35b1d Mon Sep 17 00:00:00 2001 From: gitRasheed Date: Sat, 5 Sep 2026 22:17:17 +0100 Subject: [PATCH 1/3] Keep Xor from modifying its bitmap argument An array or run receiver XORed with a bitmap container ran the in-place operation on the argument's container and adopted it, so a.Xor(b) changed b and left both bitmaps sharing storage. Compute the result on the receiver's side instead. The test covers every in-place operation and container pair. --- arraycontainer.go | 2 +- inplace_ownership_test.go | 86 +++++++++++++++++++++++++++++++++++++++ runcontainer.go | 2 +- 3 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 inplace_ownership_test.go diff --git a/arraycontainer.go b/arraycontainer.go index 84075c68..c03b135e 100644 --- a/arraycontainer.go +++ b/arraycontainer.go @@ -645,7 +645,7 @@ func (ac *arrayContainer) ixorArray(value2 *arrayContainer) container { } func (ac *arrayContainer) ixorBitmap(value2 *bitmapContainer) container { - return value2.ixor(ac) + return value2.xorArray(ac) } func (ac *arrayContainer) ixorRun16(value2 *runContainer16) container { diff --git a/inplace_ownership_test.go b/inplace_ownership_test.go new file mode 100644 index 00000000..eef0f7bb --- /dev/null +++ b/inplace_ownership_test.go @@ -0,0 +1,86 @@ +package roaring + +import "testing" + +// Every in-place set operation must leave its argument untouched, and the +// two bitmaps must not share storage afterwards. +func TestInPlaceOpsPreserveArgument(t *testing.T) { + const hi = 1 << 16 + dense := func(base uint32) *Bitmap { + b := New() + for i := uint32(0); i < 65536; i += 2 { + b.Add(base | i) + } + return b + } + run := func(base uint32) *Bitmap { + b := New() + b.AddRange(uint64(base)+100, uint64(base)+60000) + return b + } + twoKeys := func(b *Bitmap) *Bitmap { + b = b.Clone() + b.AddRange(hi, hi+5) + return b + } + shapes := []struct { + name string + bm *Bitmap + }{ + {"empty", New()}, {"array", BitmapOf(1, 3, 65535)}, {"bitmap", dense(0)}, {"run", run(0)}, + {"array+key", twoKeys(BitmapOf(2, 4))}, {"run+key", twoKeys(run(0))}, + {"hi-array", BitmapOf(hi|1, hi|3)}, {"hi-bitmap", dense(hi)}, {"hi-run", run(hi)}, + } + ops := []struct { + name string + inPlace func(a, b *Bitmap) + pure func(a, b *Bitmap) *Bitmap + }{ + {"Or", func(a, b *Bitmap) { a.Or(b) }, Or}, + {"And", func(a, b *Bitmap) { a.And(b) }, And}, + {"AndNot", func(a, b *Bitmap) { a.AndNot(b) }, AndNot}, + {"Xor", func(a, b *Bitmap) { a.Xor(b) }, Xor}, + } + for _, op := range ops { + for _, left := range shapes { + for _, right := range shapes { + l, r := left.bm, right.bm + for _, cow := range []bool{false, true} { + name := op.name + "/" + left.name + "/" + right.name + a, b := l.Clone(), r.Clone() + a.SetCopyOnWrite(cow) + b.SetCopyOnWrite(cow) + snapshot := a.Clone() + want := op.pure(l, r) + op.inPlace(a, b) + if !snapshot.Equals(l) { + t.Fatalf("%s: a clone of the receiver changed (copy-on-write %v)", name, cow) + } + if !a.Equals(want) { + t.Fatalf("%s: wrong result", name) + } + if err := a.Validate(); err != nil { + t.Fatalf("%s: %v", name, err) + } + if !b.Equals(r) { + t.Fatalf("%s: argument modified (copy-on-write %v)", name, cow) + } + it := r.Iterator() + for n := 0; n < 50 && it.HasNext(); n++ { + a.Remove(it.Next()) + } + a.Add(65533) + a.AddRange(hi+70000, hi+70050) + if !b.Equals(r) { + t.Fatalf("%s: editing the result changed the argument (copy-on-write %v)", name, cow) + } + } + } + a := left.bm.Clone() + op.inPlace(a, a) + if !a.Equals(op.pure(left.bm, left.bm)) { + t.Fatalf("%s/%s: wrong result for a bitmap applied to itself", op.name, left.name) + } + } + } +} diff --git a/runcontainer.go b/runcontainer.go index 7c369b75..7df434cd 100644 --- a/runcontainer.go +++ b/runcontainer.go @@ -2452,7 +2452,7 @@ func (rc *runContainer16) ixorArray(value2 *arrayContainer) container { } func (rc *runContainer16) ixorBitmap(value2 *bitmapContainer) container { - return value2.ixor(rc) + return rc.toBitmapContainer().ixorBitmap(value2) } func (rc *runContainer16) ixorRunContainer16(value2 *runContainer16) container { From 0765837a147847907ff701bb6010aeda6d3bf9c3 Mon Sep 17 00:00:00 2001 From: gitRasheed Date: Sat, 5 Sep 2026 22:17:18 +0100 Subject: [PATCH 2/3] Clone containers inserted by 64-bit Xor and guard self-XOR Keys present only in the argument were inserted by pointer, so editing the receiver afterwards changed the argument. Clone them, as Or does. XORing a bitmap with itself removed entries from the structure being iterated and could panic; clear the receiver instead, as the 32-bit Xor does. --- roaring64/inplace_ownership_test.go | 86 +++++++++++++++++++++++++++++ roaring64/roaring64.go | 6 +- 2 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 roaring64/inplace_ownership_test.go diff --git a/roaring64/inplace_ownership_test.go b/roaring64/inplace_ownership_test.go new file mode 100644 index 00000000..aaec3fc6 --- /dev/null +++ b/roaring64/inplace_ownership_test.go @@ -0,0 +1,86 @@ +package roaring64 + +import "testing" + +// Every in-place set operation must leave its argument untouched, and the +// two bitmaps must not share storage afterwards. +func TestInPlaceOpsPreserveArgument(t *testing.T) { + const hi = 1 << 32 + dense := func(base uint64) *Bitmap { + b := New() + for i := uint64(0); i < 65536; i += 2 { + b.Add(base | i) + } + return b + } + run := func(base uint64) *Bitmap { + b := New() + b.AddRange(base+100, base+60000) + return b + } + twoKeys := func(b *Bitmap) *Bitmap { + b = b.Clone() + b.AddRange(hi, hi+5) + return b + } + shapes := []struct { + name string + bm *Bitmap + }{ + {"empty", New()}, {"array", BitmapOf(1, 3, 65535)}, {"bitmap", dense(0)}, {"run", run(0)}, + {"array+key", twoKeys(BitmapOf(2, 4))}, {"run+key", twoKeys(run(0))}, + {"hi-array", BitmapOf(hi|1, hi|3)}, {"hi-bitmap", dense(hi)}, {"hi-run", run(hi)}, + } + ops := []struct { + name string + inPlace func(a, b *Bitmap) + pure func(a, b *Bitmap) *Bitmap + }{ + {"Or", func(a, b *Bitmap) { a.Or(b) }, Or}, + {"And", func(a, b *Bitmap) { a.And(b) }, And}, + {"AndNot", func(a, b *Bitmap) { a.AndNot(b) }, AndNot}, + {"Xor", func(a, b *Bitmap) { a.Xor(b) }, Xor}, + } + for _, op := range ops { + for _, left := range shapes { + for _, right := range shapes { + l, r := left.bm, right.bm + for _, cow := range []bool{false, true} { + name := op.name + "/" + left.name + "/" + right.name + a, b := l.Clone(), r.Clone() + a.SetCopyOnWrite(cow) + b.SetCopyOnWrite(cow) + snapshot := a.Clone() + want := op.pure(l, r) + op.inPlace(a, b) + if !snapshot.Equals(l) { + t.Fatalf("%s: a clone of the receiver changed (copy-on-write %v)", name, cow) + } + if !a.Equals(want) { + t.Fatalf("%s: wrong result", name) + } + if err := a.Validate(); err != nil { + t.Fatalf("%s: %v", name, err) + } + if !b.Equals(r) { + t.Fatalf("%s: argument modified (copy-on-write %v)", name, cow) + } + it := r.Iterator() + for n := 0; n < 50 && it.HasNext(); n++ { + a.Remove(it.Next()) + } + a.Add(65533) + a.AddRange(hi+70000, hi+70050) + if !b.Equals(r) { + t.Fatalf("%s: editing the result changed the argument (copy-on-write %v)", name, cow) + } + } + } + a := left.bm.Clone() + op.inPlace(a, a) + if !a.Equals(op.pure(left.bm, left.bm)) { + t.Fatalf("%s/%s: wrong result for a bitmap applied to itself", op.name, left.name) + } + } + } +} diff --git a/roaring64/roaring64.go b/roaring64/roaring64.go index 143209de..cea0da3e 100644 --- a/roaring64/roaring64.go +++ b/roaring64/roaring64.go @@ -632,6 +632,10 @@ main: // Xor computes the symmetric difference between two bitmaps and stores the result in the current bitmap func (rb *Bitmap) Xor(x2 *Bitmap) { + if rb == x2 { + rb.Clear() + return + } pos1 := 0 pos2 := 0 length1 := rb.highlowcontainer.size() @@ -646,7 +650,7 @@ func (rb *Bitmap) Xor(x2 *Bitmap) { break } } else if s1 > s2 { - c := x2.highlowcontainer.getWritableContainerAtIndex(pos2) + c := x2.highlowcontainer.getContainerAtIndex(pos2).Clone() rb.highlowcontainer.insertNewKeyValueAt(pos1, x2.highlowcontainer.getKeyAtIndex(pos2), c) length1++ pos1++ From 345acd1320df68153d95365a01f7e1f6628464c1 Mon Sep 17 00:00:00 2001 From: gitRasheed Date: Sat, 5 Sep 2026 22:17:21 +0100 Subject: [PATCH 3/3] Compute 64-bit Xor in place for matching keys Reuse the receiver's writable container instead of allocating a fresh symmetric difference for every matching key. --- roaring64/roaring64.go | 4 +- roaring64/xor_inplace_test.go | 93 +++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 roaring64/xor_inplace_test.go diff --git a/roaring64/roaring64.go b/roaring64/roaring64.go index cea0da3e..61cfe9aa 100644 --- a/roaring64/roaring64.go +++ b/roaring64/roaring64.go @@ -656,8 +656,8 @@ func (rb *Bitmap) Xor(x2 *Bitmap) { pos1++ pos2++ } else { - // TODO: couple be computed in-place for reduced memory usage - c := roaring.Xor(rb.highlowcontainer.getContainerAtIndex(pos1), x2.highlowcontainer.getContainerAtIndex(pos2)) + c := rb.highlowcontainer.getWritableContainerAtIndex(pos1) + c.Xor(x2.highlowcontainer.getContainerAtIndex(pos2)) if !c.IsEmpty() { rb.highlowcontainer.setContainerAtIndex(pos1, c) pos1++ diff --git a/roaring64/xor_inplace_test.go b/roaring64/xor_inplace_test.go new file mode 100644 index 00000000..08c65b0c --- /dev/null +++ b/roaring64/xor_inplace_test.go @@ -0,0 +1,93 @@ +package roaring64 + +import "testing" + +func TestXorInPlaceDenseAllocations(t *testing.T) { + x, y := New(), New() + for i := uint64(0); i < 65536; i++ { + if i%2 == 0 { + x.Add(1<<40 | i) + } + if i%4 < 2 { + y.Add(1<<40 | i) + } + } + original := x.Clone() + allocs := testing.AllocsPerRun(10, func() { + x.Xor(y) + x.Xor(y) + }) + if allocs != 0 { + t.Fatalf("dense in-place XOR allocated %g times", allocs) + } + if !x.Equals(original) { + t.Fatal("two XORs must restore the receiver") + } +} + +func BenchmarkXorInPlace64(b *testing.B) { + for _, kind := range []string{"dense", "array", "run", "disjoint-keys"} { + x, y := New(), New() + for high := uint64(1); high <= 4; high++ { + base := high << 40 + switch kind { + case "dense": + for i := uint64(0); i < 65536; i++ { + if i%2 == 0 { + x.Add(base | i) + } + if i%4 < 2 { + y.Add(base | i) + } + } + case "array": + for i := uint64(0); i < 128; i++ { + if i%2 == 0 { + x.Add(base | i) + } + if i%4 < 2 { + y.Add(base | i) + } + } + case "run": + x.AddRange(base+100, base+30000) + y.AddRange(base+20000, base+50000) + case "disjoint-keys": + x.Add(base | 1) + y.Add((base - 1<<32) | 2) + } + } + for _, clone := range []bool{false, true} { + name := kind + "/reuse" + if clone { + name = kind + "/clone-and-xor" + } + b.Run(name, func(b *testing.B) { + x, y := x.Clone(), y.Clone() + original := x.Clone() + want := Xor(x, y) + b.ReportAllocs() + var result *Bitmap + for b.Loop() { + if clone { + result = x.Clone() + result.Xor(y) + } else { + x.Xor(y) + x.Xor(y) + } + } + if clone { + if !result.Equals(want) { + b.Fatal("wrong XOR result") + } + } else { + if !x.Equals(original) { + b.Fatal("two XORs did not restore the receiver") + } + b.ReportMetric(2, "xors/op") + } + }) + } + } +}