From 79ebb60d94aa2fe21b3820fa633cb0a5d9bc4cc9 Mon Sep 17 00:00:00 2001 From: Akron Date: Tue, 27 Oct 2026 16:43:05 +0100 Subject: [PATCH 1/2] Fix NextAbsentValue and PreviousAbsentValue --- arraycontainer.go | 2 +- arraycontainer_test.go | 24 +++++++++ bitmapcontainer.go | 115 +++++++++++----------------------------- bitmapcontainer_test.go | 31 +++++++++++ roaring.go | 41 ++++++++++---- roaring_test.go | 42 +++++++++++++++ 6 files changed, 160 insertions(+), 95 deletions(-) diff --git a/arraycontainer.go b/arraycontainer.go index 84075c68..47fc6cc1 100644 --- a/arraycontainer.go +++ b/arraycontainer.go @@ -1193,7 +1193,7 @@ func (ac *arrayContainer) nextAbsentValue(target uint16) int { } if low == cardinality-1 { - return int(ac.content[cardinality-1] + 1) + return int(ac.content[cardinality-1]) + 1 } return int(ac.content[low] + 1) diff --git a/arraycontainer_test.go b/arraycontainer_test.go index 5abe5668..9d517ded 100644 --- a/arraycontainer_test.go +++ b/arraycontainer_test.go @@ -642,6 +642,30 @@ func TestPreviousValueArray(t *testing.T) { }) } +// The next absent value after the last value 65535 must not wrap +// around to 0 (uint16 arithmetic). + +func TestArrayContainerNextAbsentValueAtEnd(t *testing.T) { + t.Run("One value at the end", func(t *testing.T) { + ac := newArrayContainer() + ac.iadd(65535) + assert.Equal(t, 1<<16, ac.nextAbsentValue(65535)) + }) + + t.Run("Two values at the end", func(t *testing.T) { + ac := newArrayContainer() + ac.iadd(65534) + ac.iadd(65535) + assert.Equal(t, 1<<16, ac.nextAbsentValue(65534)) + assert.Equal(t, 1<<16, ac.nextAbsentValue(65535)) + }) + + t.Run("In a bitmap", func(t *testing.T) { + bmp := BitmapOf(1, 65535) + assert.Equal(t, int64(65536), bmp.NextAbsentValue(65535)) + }) +} + func TestPreviousAbsentValueArray(t *testing.T) { t.Run("Java Port 1", func(t *testing.T) { // [Example 1] https://github.com/RoaringBitmap/RoaringBitmap/blob/5235aa62c32fa3bf7fae40a562e3edc75f61be4e/RoaringBitmap/src/test/java/org/roaringbitmap/TestArrayContainer.java#L793 diff --git a/bitmapcontainer.go b/bitmapcontainer.go index 6ca8f679..86365276 100644 --- a/bitmapcontainer.go +++ b/bitmapcontainer.go @@ -1393,50 +1393,27 @@ func (bc *bitmapContainer) nextValue(target uint16) int { // nextAbsentValue returns the next absent value. // if the target is out of bounds a -1 is returned func (bc *bitmapContainer) nextAbsentValue(target uint16) int { - if bc.cardinality == 0 { - return -1 + x := uint(target >> 6) + length := uint(len(bc.bitmap)) + if x >= length { + return 1 << 16 } - var ( - x = target >> 6 - length = uint(len(bc.bitmap)) - ) - if uint(x) >= length { - return -1 - } - w := bc.bitmap[x] - w = w >> uint(target%64) - if w == 0 { - return int(target) + // the zeros of the word that are at or after the target + w := (^bc.bitmap[x]) >> uint(target%64) + if w != 0 { + return int(target) + bits.TrailingZeros64(w) } - // Check if all 1's - // if statement - we skip the if we have all ones [1,1,1,1...1] - if ^w != 0 { - - if bits.TrailingZeros64(w) > 0 { - // we have something like [X,Y,Z, 0,0,0]. This means the target bit is zero - return int(target) - } - - // other wise something like [X,Y,0,1,1,1..1], where x and y can be either 1 or 0. - - trailing := bits.TrailingZeros64(^w) - return int(target) + trailing - - } x++ - for ; uint(x) < length; x++ { - if bc.bitmap[x] == 0 { - return int(x * 64) - } - if ^bc.bitmap[x] != 0 { - trailing := bits.TrailingZeros64(^bc.bitmap[x]) - return int(x*64) + trailing + for ; x < length; x++ { + if inv := ^bc.bitmap[x]; inv != 0 { + return int(x*64) + bits.TrailingZeros64(inv) } - } - return -1 + + // All words up to the end are full. + return 1 << 16 } // previousValue returns either the `target` if found or the previous largest value. @@ -1460,62 +1437,30 @@ func (bc *bitmapContainer) previousValue(target uint16) int { // previousAbsentValue returns the next absent value. func (bc *bitmapContainer) previousAbsentValue(target uint16) int { - if bc.cardinality == 0 { + x := int(target >> 6) + if x >= len(bc.bitmap) { return -1 } - var ( - x = target >> 6 - length = uint(len(bc.bitmap)) - ) - if uint(x) >= length { - return -1 - } - w := bc.bitmap[x] - shifted := w >> uint(target%64) - if shifted == 0 { - return int(target) + // Look at the target's word first: invert it, then shift the bits above + // the target out to the left, so that the target is bit 63 and the bits + // in front of it follow below. The zeros that the shift brings in at the + // bottom are zeros of the inverse, so they never count as absent values. + w := (^bc.bitmap[x]) << (63 - uint(target%64)) + if w != 0 { + return int(target) - bits.LeadingZeros64(w) } - // Check if all 1's - // if statement - we skip if we have all ones [1,1,1,1...1] as no value is absent - if ^shifted != 0 { - - if bits.TrailingZeros64(shifted) > 0 { - // we have something like shifted=[X,Y,Z,..., 0,0,0]. This means the target bit is zero - return int(target) - } - - // The rotate will rotate the target bit into the leading position. - // We know the target bit is not zero because of the TrailingZeros64 check above - // We then shift the target bit out of the way. - // Assume a structure like an original structure like [X,Y,Z,..., Target, A, B,C...] - // shifted will be [X,Y,Z...Target] - // shiftedRotated will be [A,B,C....] - // If LeadingZeros64 > 0 then A is zero, if not at least A is 1 return - // Else count the number of ones's until a 0 - shiftedRotated := bits.RotateLeft64(w, int(64-uint(target%64))-1) << 1 - leadingZeros := bits.LeadingZeros64(shiftedRotated) - if leadingZeros > 0 { - return int(target) - 1 + // Nothing absent at or in front of the target in this word. Go on with the + // words in front of it, downwards. + for x--; x >= 0; x-- { + if inv := ^bc.bitmap[x]; inv != 0 { + // the last 1 bit of the inverse is the highest absent value of the word + return x*64 + 63 - bits.LeadingZeros64(inv) } - leadingOnes := bits.LeadingZeros64(^shiftedRotated) - if leadingOnes > 0 { - return int(target) - leadingOnes - 1 - } - } - x++ - for ; uint(x) < length; x++ { - if bc.bitmap[x] == 0 { - return int(x * 64) - } - if ^bc.bitmap[x] != 0 { - trailing := bits.TrailingZeros64(^bc.bitmap[x]) - return int(x*64) + trailing - } - } + // All words down to the start are full. return -1 } diff --git a/bitmapcontainer_test.go b/bitmapcontainer_test.go index 48e423dc..46eabedb 100644 --- a/bitmapcontainer_test.go +++ b/bitmapcontainer_test.go @@ -412,6 +412,37 @@ func TestNextAbsent(t *testing.T) { assert.Equal(t, v, 65536) } +// The scans of the absent value must not stop at the border of a +// 64 bit word. +func TestBitmapContainerAbsentValueAcrossWords(t *testing.T) { + t.Run("Next absent value after a run of ones that reaches the end of a word", func(t *testing.T) { + bc := newBitmapContainer() + for i := 0; i < 5000; i++ { + bc.iadd(uint16(i)) + } + assert.Equal(t, 5000, bc.nextAbsentValue(10)) + assert.Equal(t, 5000, bc.nextAbsentValue(70)) + }) + + t.Run("Next absent value of a full container", func(t *testing.T) { + bc := newBitmapContainer() + for i := 0; i < 1<<16; i++ { + bc.iadd(uint16(i)) + } + assert.Equal(t, 1<<16, bc.nextAbsentValue(0)) + assert.Equal(t, 1<<16, bc.nextAbsentValue(64)) + }) + + t.Run("Previous absent value goes down through the earlier words", func(t *testing.T) { + bc := newBitmapContainer() + for i := 1; i <= 5000; i++ { + bc.iadd(uint16(i)) + } + assert.Equal(t, 0, bc.previousAbsentValue(5000)) + assert.Equal(t, 0, bc.previousAbsentValue(70)) + }) +} + func TestBitMapContainerValidate(t *testing.T) { bc := newBitmapContainer() diff --git a/roaring.go b/roaring.go index b47c5404..c809e7a6 100644 --- a/roaring.go +++ b/roaring.go @@ -2320,28 +2320,41 @@ func (rb *Bitmap) NextAbsentValue(target uint32) int64 { return int64(target) } + // A value is made of the container key (the upper 16 bits) and the value in + // the container (the lower 16 bits). The container only knows the lower + // 16 bits; 1<<16 is its answer for "every value up to its end is present". container := rb.highlowcontainer.getContainer(containerKey) nextValue = int64(container.nextAbsentValue(query)) for { if nextValue != (1 << 16) { - return int64(combineLoHi32(uint32(nextValue), keyspace)) + // Found. keyspace is the key already shifted into the upper 16 bits, + // so it is only OR-ed to the lower bits. + return int64(uint32(nextValue) | keyspace) } if containerIndex == rb.highlowcontainer.size()-1 { - val, err := container.safeMaximum() - if err == nil { + // It is the last container. The next value is the first one of the + // next key, which is absent because there is no such container. + // Only the last key, 0xFFFF, has no next key: nothing is absent. + // (safeMaximum() returns err == nil when the container has values, + // so testing it for "empty" gave -1 for every full last container.) + if containerKey == 0xFFFF { return -1 } - return int64(val) + 1 + return int64(containerKey+1) << 16 } containerIndex++ nextContainerKey := rb.highlowcontainer.getKeyAtIndex(containerIndex) - if containerKey < nextContainerKey { + if containerKey+1 < nextContainerKey { // There is a gap between keys // Just increment the current key and shift to get HoB return int64(containerKey+1) << 16 } + + // The next container follows directly: search it from its start. Its + // key is the upper part of the result, so keyspace has to follow. containerKey = nextContainerKey + keyspace = uint32(containerKey) << 16 container = rb.highlowcontainer.getContainer(containerKey) nextValue = int64(container.nextAbsentValue(0)) } @@ -2381,16 +2394,22 @@ func (rb *Bitmap) PreviousAbsentValue(target uint32) int64 { prevValue = int64(container.previousAbsentValue(query)) for { if prevValue != -1 { - return int64(combineLoHi32(uint32(prevValue), keyspace)) + // Found. keyspace is the key already shifted into the upper 16 bits, + // so it is only OR-ed to the lower bits. + return int64(uint32(prevValue) | keyspace) } if containerIndex == 0 { - val, err := container.safeMinimum() - if err == nil { + // It is the first container. The value before it is the last one of + // the previous key, which is absent because there is no such + // container. Only key 0 has no previous key: nothing is absent. + // (safeMinimum() returns err == nil when the container has values, + // so testing it for "empty" gave -1 for every full first container.) + if containerKey == 0 { // OR panic, Java panics return -1 } - return int64(val) - 1 + return (int64(containerKey) << 16) - 1 } containerIndex-- nextContainerKey := rb.highlowcontainer.getKeyAtIndex(containerIndex) @@ -2399,7 +2418,11 @@ func (rb *Bitmap) PreviousAbsentValue(target uint32) int64 { // Just decrement the current key and shift to get HoB of the missing container return (int64(containerKey) << 16) - 1 } + + // The previous container follows directly: search it from its end. Its + // key is the upper part of the result, so keyspace has to follow. containerKey = nextContainerKey + keyspace = uint32(containerKey) << 16 container = rb.highlowcontainer.getContainer(containerKey) highestPossible16 := (1 << 16) - 1 prevValue = int64(container.previousAbsentValue(uint16(highestPossible16))) diff --git a/roaring_test.go b/roaring_test.go index 5ccbdd32..c19b6972 100644 --- a/roaring_test.go +++ b/roaring_test.go @@ -3708,6 +3708,48 @@ func TestNextAndPreviousValue(t *testing.T) { }) } +// The bitmap containers are tested in bitmapcontainer_test.go, the array +// containers in arraycontainer_test.go. +func TestNextAndPreviousAbsentValueContainerKeys(t *testing.T) { + t.Run("High bits of the result for a key other than 0", func(t *testing.T) { + bmp := BitmapOf(65537) + assert.Equal(t, int64(65536), bmp.NextAbsentValue(65536)) + assert.Equal(t, int64(65538), bmp.NextAbsentValue(65537)) + assert.Equal(t, int64(65536), bmp.PreviousAbsentValue(65537)) + assert.Equal(t, int64(65538), bmp.PreviousAbsentValue(65538)) + }) + + t.Run("Key follows the search into the previous container", func(t *testing.T) { + // keys 1 and 2: with the keys 0 and 1 a wrong key is not visible + bmp := New() + bmp.Add(65536) + bmp.Add(131071) + bmp.AddRange(131072, 196608) // a full container follows + assert.Equal(t, int64(131070), bmp.PreviousAbsentValue(196607)) + }) + + t.Run("Full last container", func(t *testing.T) { + bmp := New() + bmp.AddRange(0, 65536) + assert.Equal(t, int64(65536), bmp.NextAbsentValue(0)) + assert.Equal(t, int64(65536), bmp.NextAbsentValue(65535)) + }) + + t.Run("Full first container", func(t *testing.T) { + bmp := New() + bmp.AddRange(65536, 131072) + assert.Equal(t, int64(65535), bmp.PreviousAbsentValue(131071)) + assert.Equal(t, int64(65535), bmp.PreviousAbsentValue(65536)) + }) + + t.Run("Full containers next to each other", func(t *testing.T) { + bmp := New() + bmp.AddRange(0, 131072) + assert.Equal(t, int64(131072), bmp.NextAbsentValue(0)) + assert.Equal(t, int64(131072), bmp.NextAbsentValue(65536)) + }) +} + func BenchmarkFromDense(b *testing.B) { testDense(func(name string, rb *Bitmap) { dense := make([]uint64, rb.DenseSize()) From e2916f93d3734adc3e129d6695e3db1d0d7f45da Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Tue, 27 Oct 2026 18:10:27 -0400 Subject: [PATCH 2/2] Trim comments on the absent-value fix keyspace is already key<<16. 1<<16 and -1 mean the container is exhausted. Drop the notes that retell the old safeMinimum and safeMaximum checks, and correct nextAbsentValue's sentinel. --- arraycontainer_test.go | 4 +--- bitmapcontainer.go | 18 +++++------------- bitmapcontainer_test.go | 2 -- roaring.go | 23 ++--------------------- roaring_test.go | 6 ++---- 5 files changed, 10 insertions(+), 43 deletions(-) diff --git a/arraycontainer_test.go b/arraycontainer_test.go index 9d517ded..d9da91a9 100644 --- a/arraycontainer_test.go +++ b/arraycontainer_test.go @@ -642,9 +642,7 @@ func TestPreviousValueArray(t *testing.T) { }) } -// The next absent value after the last value 65535 must not wrap -// around to 0 (uint16 arithmetic). - +// uint16(65535)+1 wraps to 0. func TestArrayContainerNextAbsentValueAtEnd(t *testing.T) { t.Run("One value at the end", func(t *testing.T) { ac := newArrayContainer() diff --git a/bitmapcontainer.go b/bitmapcontainer.go index 86365276..56c2bf9c 100644 --- a/bitmapcontainer.go +++ b/bitmapcontainer.go @@ -1390,8 +1390,8 @@ func (bc *bitmapContainer) nextValue(target uint16) int { return bc.NextSetBit(uint(target)) } -// nextAbsentValue returns the next absent value. -// if the target is out of bounds a -1 is returned +// nextAbsentValue returns the smallest absent value >= target. +// The result is in 0..65535, or 1<<16 when every such value is present. func (bc *bitmapContainer) nextAbsentValue(target uint16) int { x := uint(target >> 6) length := uint(len(bc.bitmap)) @@ -1399,7 +1399,6 @@ func (bc *bitmapContainer) nextAbsentValue(target uint16) int { return 1 << 16 } - // the zeros of the word that are at or after the target w := (^bc.bitmap[x]) >> uint(target%64) if w != 0 { return int(target) + bits.TrailingZeros64(w) @@ -1412,7 +1411,6 @@ func (bc *bitmapContainer) nextAbsentValue(target uint16) int { } } - // All words up to the end are full. return 1 << 16 } @@ -1435,32 +1433,26 @@ func (bc *bitmapContainer) previousValue(target uint16) int { return bc.uPrevSetBit(uint(target)) } -// previousAbsentValue returns the next absent value. +// previousAbsentValue returns the largest absent value <= target, +// or -1 when every such value is present. func (bc *bitmapContainer) previousAbsentValue(target uint16) int { x := int(target >> 6) if x >= len(bc.bitmap) { return -1 } - // Look at the target's word first: invert it, then shift the bits above - // the target out to the left, so that the target is bit 63 and the bits - // in front of it follow below. The zeros that the shift brings in at the - // bottom are zeros of the inverse, so they never count as absent values. + // Shift left by 63-(target%64) so LeadingZeros ignores bits shifted in. w := (^bc.bitmap[x]) << (63 - uint(target%64)) if w != 0 { return int(target) - bits.LeadingZeros64(w) } - // Nothing absent at or in front of the target in this word. Go on with the - // words in front of it, downwards. for x--; x >= 0; x-- { if inv := ^bc.bitmap[x]; inv != 0 { - // the last 1 bit of the inverse is the highest absent value of the word return x*64 + 63 - bits.LeadingZeros64(inv) } } - // All words down to the start are full. return -1 } diff --git a/bitmapcontainer_test.go b/bitmapcontainer_test.go index 46eabedb..ba1f28c1 100644 --- a/bitmapcontainer_test.go +++ b/bitmapcontainer_test.go @@ -412,8 +412,6 @@ func TestNextAbsent(t *testing.T) { assert.Equal(t, v, 65536) } -// The scans of the absent value must not stop at the border of a -// 64 bit word. func TestBitmapContainerAbsentValueAcrossWords(t *testing.T) { t.Run("Next absent value after a run of ones that reaches the end of a word", func(t *testing.T) { bc := newBitmapContainer() diff --git a/roaring.go b/roaring.go index c809e7a6..ce6bc1db 100644 --- a/roaring.go +++ b/roaring.go @@ -2320,24 +2320,15 @@ func (rb *Bitmap) NextAbsentValue(target uint32) int64 { return int64(target) } - // A value is made of the container key (the upper 16 bits) and the value in - // the container (the lower 16 bits). The container only knows the lower - // 16 bits; 1<<16 is its answer for "every value up to its end is present". + // keyspace is already key<<16. 1<<16 means the container is exhausted. container := rb.highlowcontainer.getContainer(containerKey) nextValue = int64(container.nextAbsentValue(query)) for { if nextValue != (1 << 16) { - // Found. keyspace is the key already shifted into the upper 16 bits, - // so it is only OR-ed to the lower bits. return int64(uint32(nextValue) | keyspace) } if containerIndex == rb.highlowcontainer.size()-1 { - // It is the last container. The next value is the first one of the - // next key, which is absent because there is no such container. - // Only the last key, 0xFFFF, has no next key: nothing is absent. - // (safeMaximum() returns err == nil when the container has values, - // so testing it for "empty" gave -1 for every full last container.) if containerKey == 0xFFFF { return -1 } @@ -2351,8 +2342,6 @@ func (rb *Bitmap) NextAbsentValue(target uint32) int64 { return int64(containerKey+1) << 16 } - // The next container follows directly: search it from its start. Its - // key is the upper part of the result, so keyspace has to follow. containerKey = nextContainerKey keyspace = uint32(containerKey) << 16 container = rb.highlowcontainer.getContainer(containerKey) @@ -2390,21 +2379,15 @@ func (rb *Bitmap) PreviousAbsentValue(target uint32) int64 { return int64(target) } + // keyspace is already key<<16. -1 means the container is exhausted. container := rb.highlowcontainer.getContainer(containerKey) prevValue = int64(container.previousAbsentValue(query)) for { if prevValue != -1 { - // Found. keyspace is the key already shifted into the upper 16 bits, - // so it is only OR-ed to the lower bits. return int64(uint32(prevValue) | keyspace) } if containerIndex == 0 { - // It is the first container. The value before it is the last one of - // the previous key, which is absent because there is no such - // container. Only key 0 has no previous key: nothing is absent. - // (safeMinimum() returns err == nil when the container has values, - // so testing it for "empty" gave -1 for every full first container.) if containerKey == 0 { // OR panic, Java panics return -1 @@ -2419,8 +2402,6 @@ func (rb *Bitmap) PreviousAbsentValue(target uint32) int64 { return (int64(containerKey) << 16) - 1 } - // The previous container follows directly: search it from its end. Its - // key is the upper part of the result, so keyspace has to follow. containerKey = nextContainerKey keyspace = uint32(containerKey) << 16 container = rb.highlowcontainer.getContainer(containerKey) diff --git a/roaring_test.go b/roaring_test.go index c19b6972..b5e9e8ba 100644 --- a/roaring_test.go +++ b/roaring_test.go @@ -3708,8 +3708,6 @@ func TestNextAndPreviousValue(t *testing.T) { }) } -// The bitmap containers are tested in bitmapcontainer_test.go, the array -// containers in arraycontainer_test.go. func TestNextAndPreviousAbsentValueContainerKeys(t *testing.T) { t.Run("High bits of the result for a key other than 0", func(t *testing.T) { bmp := BitmapOf(65537) @@ -3720,11 +3718,11 @@ func TestNextAndPreviousAbsentValueContainerKeys(t *testing.T) { }) t.Run("Key follows the search into the previous container", func(t *testing.T) { - // keys 1 and 2: with the keys 0 and 1 a wrong key is not visible + // Keys 1 and 2: key 0 would hide a missing high-half update. bmp := New() bmp.Add(65536) bmp.Add(131071) - bmp.AddRange(131072, 196608) // a full container follows + bmp.AddRange(131072, 196608) assert.Equal(t, int64(131070), bmp.PreviousAbsentValue(196607)) })