Skip to content

Fix SumBigValues overflow without serializing large sums - #590

Open
perfloop-agent wants to merge 2 commits into
RoaringBitmap:masterfrom
perfloop-oss:perfloop-pr-open-t7qdrbfarj
Open

perfloop-agent wants to merge 2 commits into
RoaringBitmap:masterfrom
perfloop-oss:perfloop-pr-open-t7qdrbfarj

Conversation

@perfloop-agent

Copy link
Copy Markdown
Contributor

Description

Fix SumBigValues overflow by weighting cardinalities in big.Int, while keeping expensive intersections parallel and avoiding worker setup for small sums.

Supersedes our #552, which made large sums slower because it runs the per-plane intersections serially. In a local loop equivalent to that serial path, 2M-row sums were 45–70% slower than master at -cpu 2 (three pairs each for 16-, 63- and 64-bit values). This PR keeps #552's overflow fix and keeps large and scattered sums parallel.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Performance improvement
  • Test improvements

Changes Made

  • Collect unshifted cardinalities, then add ordinary planes and subtract the sign plane with a reusable big.Int.
  • Run serially at GOMAXPROCS=1 or below 256 plane-container pairs, counting the sign plane. Otherwise, at most GOMAXPROCS workers take planes dynamically. Nil filters and returned counts keep their behavior.
  • Include Avoid goroutine fan-out in SumBigValues #552's TestSumBigValues for wide positive, negative and filtered sums, plus a parallel-path regression.

Testing

The Go test runner passed the roaring64 suite, including the wide signed-sum and parallel-path regressions. A separate differential check, not part of this PR, compared 300 fixed-seed signed fixtures up to 256 bits against a per-column big.Int total at GOMAXPROCS 1, 2 and 4, including dense, scattered, empty and absent-ID filters.

Formatting

Changed Go files are gofmt-formatted, and go tool unconvert ./... reports no unnecessary conversions.

Fuzzing

Not run. The repository's fuzz targets do not cover roaring64 sums.

Performance Impact

Selected native rows from the first measured pair at each CPU setting (local run): the first nine rows are before, and the last nine, from the second rows128/bits63/all row, are after.

BenchmarkBSI64SumBigValues/rows128/bits63/all         	   16138	     22615 ns/op	    5289 B/op	     132 allocs/op
BenchmarkBSI64SumBigValues/rows100000/bits63/all      	    8564	     38585 ns/op	    5288 B/op	     132 allocs/op
BenchmarkBSI64SumBigValues/rows2000000/bits63/all     	     568	    639347 ns/op	    5288 B/op	     132 allocs/op
BenchmarkBSI64SumBigValues/rows2000000/bits63/all-2     	    1022	    341959 ns/op	    5288 B/op	     132 allocs/op
BenchmarkBSI64SumBigValues/rows2000000/bits63/scattered100000-2     	     100	   3367589 ns/op	    5288 B/op	     132 allocs/op
BenchmarkBSI64SumBigValues/rows2000000/bits63/scattered262143-2     	     918	    381880 ns/op	    5288 B/op	     132 allocs/op
BenchmarkBSI64SumBigValues/rows8000000/bits63/scattered250000-2     	      38	   8328368 ns/op	    5288 B/op	     132 allocs/op
BenchmarkBSI64SumBigValues/rows2000000/bits63/all-4     	    1339	    241830 ns/op	    5288 B/op	     132 allocs/op
BenchmarkBSI64SumBigValues/rows2000000/bits63/scattered100000-4     	     184	   2030946 ns/op	    5288 B/op	     132 allocs/op
BenchmarkBSI64SumBigValues/rows128/bits63/all         	  202236	      1775 ns/op	     144 B/op	       5 allocs/op
BenchmarkBSI64SumBigValues/rows100000/bits63/all      	   19197	     18823 ns/op	     144 B/op	       5 allocs/op
BenchmarkBSI64SumBigValues/rows2000000/bits63/all     	     606	    593068 ns/op	     144 B/op	       5 allocs/op
BenchmarkBSI64SumBigValues/rows2000000/bits63/all-2     	     884	    350043 ns/op	     808 B/op	      10 allocs/op
BenchmarkBSI64SumBigValues/rows2000000/bits63/scattered100000-2     	     100	   3252573 ns/op	     808 B/op	      10 allocs/op
BenchmarkBSI64SumBigValues/rows2000000/bits63/scattered262143-2     	     778	    413110 ns/op	     808 B/op	      10 allocs/op
BenchmarkBSI64SumBigValues/rows8000000/bits63/scattered250000-2     	      36	   9598339 ns/op	     808 B/op	      10 allocs/op
BenchmarkBSI64SumBigValues/rows2000000/bits63/all-4     	    1360	    239153 ns/op	     936 B/op	      12 allocs/op
BenchmarkBSI64SumBigValues/rows2000000/bits63/scattered100000-4     	     178	   2001413 ns/op	     936 B/op	      12 allocs/op

In this first pair, the 8M-row scattered case at -cpu 2 was slower (8328368 → 9598339 ns/op); over three pairs its median was 8% faster (table below).

Local before → after medians; each row has three paired runs after an excluded warmup.

Rows / bits / filter / cpu ns/op before → after (median change) B/op before → after
128/63/all / 1 22166 → 1775 (-92%) 5289 → 144
100000/63/all / 1 40081 → 19262 (-52%) 5288 → 144
2000000/63/all / 1 622902 → 590946 (-5%) 5288 → 144
2000000/63/all / 2 351432 → 346276 (-1%) 5288 → 808
2000000/63/scattered100000 / 2 3299758 → 3252573 (-1%) 5288 → 808
2000000/63/scattered262143 / 2 386422 → 413110 (+7%) 5288 → 808
8000000/63/scattered250000 / 2 8771691 → 8062953 (-8%) 5288 → 808
2000000/63/all / 4 244255 → 239153 (-2%) 5288 → 936
2000000/63/scattered100000 / 4 2091811 → 2001413 (-4%) 5288 → 936

The largest median loss is 7% (2M rows, 262,143 scattered IDs, -cpu 2); a separate 1 s run of the same case was 4% faster.

The main experiment used Go 1.24.4 on linux/amd64, one caller, and already-built indexes. The 128-row case stores 0..127; other cases use fixed-seed unsigned 16/63-bit or signed 64-bit values. Scattered filters contain uniformly sampled distinct IDs; range filters use a contiguous prefix. Setup is outside the timer.

Prepared native test binaries ran with -test.run=^$ -test.bench=^BenchmarkBSI64SumBigValues$ -test.cpu=1 -test.benchtime=300ms -test.benchmem, separately repeated with -test.cpu=2 and -test.cpu=4, each with matching GOMAXPROCS.

Running Benchmarks

The native benchmark is in roaring64/bsi64_sum_benchmark_test.go. Reproduction recipe, one concurrency setting per process:

GOMAXPROCS=1 go test ./roaring64 -run '^$' -bench '^BenchmarkBSI64SumBigValues$' -cpu=1 -benchtime=300ms -benchmem
GOMAXPROCS=2 go test ./roaring64 -run '^$' -bench '^BenchmarkBSI64SumBigValues$' -cpu=2 -benchtime=300ms -benchmem
GOMAXPROCS=4 go test ./roaring64 -run '^$' -bench '^BenchmarkBSI64SumBigValues$' -cpu=4 -benchtime=300ms -benchmem

Breaking Changes

No public signature changes.

Additional Notes

Measurements use constructed indexes and repeated sums.

Case and evidence


Made by Perfloop. Human sponsor: Tomás Senart. To ask for a change, comment or review in plain words. How Perfloop handles pull requests.

Signed-off-by: Perfloop Agent <agent@perfloop.ai>
Signed-off-by: Tomás Senart <ts@perfloop.ai>
Signed-off-by: Perfloop Agent <agent@perfloop.ai>
Signed-off-by: Tomás Senart <ts@perfloop.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant