Skip to content

Commit 2f77f19

Browse files
committed
perf(atomicbitset): use CardinalityInRange via e2b-dev/roaring fork
Switch HasRange from Rank() to CardinalityInRange() which is O(k) in containers spanned rather than O(C) in total containers. Use e2b-dev/roaring fork that fixes an off-by-one in runContainer16.getCardinalityInRange for gaps between runs within a single container (upstream PR RoaringBitmap/roaring#521). Also: init dirty bitset in zero-size Cache path, fix stale comment in streaming_chunk.go.
1 parent 6c0f1dd commit 2f77f19

File tree

8 files changed

+20
-30
lines changed

8 files changed

+20
-30
lines changed

packages/orchestrator/go.mod

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ tool github.com/oapi-codegen/oapi-codegen/v2/cmd/oapi-codegen
1313

1414
tool github.com/vektra/mockery/v3
1515

16+
replace github.com/RoaringBitmap/roaring/v2 => github.com/e2b-dev/roaring/v2 v2.16.1-0.20260410073213-00ee8aef09f9
17+
1618
require (
1719
cloud.google.com/go/storage v1.59.2
1820
connectrpc.com/connect v1.18.1

packages/orchestrator/go.sum

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/orchestrator/pkg/sandbox/block/cache.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ func NewCache(size, blockSize int64, filePath string, dirtyFile bool) (*Cache, e
7272
size: size,
7373
blockSize: blockSize,
7474
dirtyFile: dirtyFile,
75+
dirty: atomicbitset.New(),
7576
}, nil
7677
}
7778

packages/orchestrator/pkg/sandbox/block/streaming_chunk.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,8 +271,8 @@ func (c *StreamingChunker) Slice(ctx context.Context, off, length int64) ([]byte
271271
}
272272

273273
// Use sliceDirect (no isCached check) since the waiter mechanism guarantees data is in the mmap.
274-
// With coarse dirty granularity, isCached may return false during an active fetch even though
275-
// the requested bytes have been written to the mmap.
274+
// setIsCached is called once per chunk after fetch completes, so isCached may return false
275+
// for bytes already written during an active fetch.
276276
b, cacheErr := c.cache.sliceDirect(off, length)
277277
if cacheErr != nil {
278278
timer.RecordRaw(ctx, length, chunkerAttrs.failLocalReadAgain)

packages/shared/go.mod

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ go 1.25.4
44

55
tool github.com/go-swagger/go-swagger/cmd/swagger
66

7+
replace github.com/RoaringBitmap/roaring/v2 => github.com/e2b-dev/roaring/v2 v2.16.1-0.20260410073213-00ee8aef09f9
8+
79
require (
810
cloud.google.com/go/artifactregistry v1.17.1
911
cloud.google.com/go/storage v1.59.2

packages/shared/go.sum

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/shared/pkg/atomicbitset/bitset.go

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,7 @@ func (b *Bitset) HasRange(start, end uint) bool {
2929
b.mu.RLock()
3030
defer b.mu.RUnlock()
3131

32-
if start >= end {
33-
return true
34-
}
35-
36-
rankEnd := b.bm.Rank(uint32(end - 1))
37-
var rankBefore uint64
38-
if start > 0 {
39-
rankBefore = b.bm.Rank(uint32(start - 1))
40-
}
41-
42-
return rankEnd-rankBefore == uint64(end-start)
32+
return b.bm.CardinalityInRange(uint64(start), uint64(end)) == uint64(end-start)
4333
}
4434

4535
func (b *Bitset) SetRange(start, end uint) {

packages/shared/pkg/atomicbitset/bitset_test.go

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -107,24 +107,19 @@ func TestHasRange_GapBetweenRanges(t *testing.T) {
107107
require.False(t, b.HasRange(1023, 1025), "crossing into gap must not be reported as set")
108108
}
109109

110-
// TestCardinalityInRange_LibraryBug documents an off-by-one in
111-
// roaring v2.16.0 runContainer16.getCardinalityInRange (line 2546):
112-
// "lastIdx < firstIdx-1" should be "lastIdx < firstIdx".
113-
// When the library fixes this, this test will fail and we can switch
114-
// HasRange from Rank() to CardinalityInRange().
115-
func TestCardinalityInRange_LibraryBug(t *testing.T) {
110+
// TestCardinalityInRange_GapFix verifies the fix for an off-by-one in
111+
// roaring v2.16.0 runContainer16.getCardinalityInRange (line 2546).
112+
// We use e2b-dev/roaring which includes the fix (upstream PR #521).
113+
// If this test breaks after switching back to upstream, the fix was
114+
// reverted or not yet merged.
115+
func TestCardinalityInRange_GapFix(t *testing.T) {
116116
t.Parallel()
117117
bm := roaring.New()
118118
bm.AddRange(0, 1024)
119119
bm.AddRange(2048, 3072)
120120

121-
card := bm.CardinalityInRange(1024, 2048)
122-
123-
// BUG: CardinalityInRange returns 1024 for a range with zero set bits.
124-
// When this assertion fails, the library has been fixed and HasRange
125-
// can be switched to use CardinalityInRange for O(k) instead of O(C).
126-
require.Equal(t, uint64(1024), card,
127-
"CardinalityInRange bug appears fixed — update HasRange to use it")
121+
require.Equal(t, uint64(0), bm.CardinalityInRange(1024, 2048),
122+
"gap between runs must have zero cardinality")
128123
}
129124

130125
// --- Benchmarks ---

0 commit comments

Comments
 (0)