New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds bitmap.NextMany() functionality #150

Merged
merged 1 commit into from Feb 26, 2018

Conversation

Projects
None yet
3 participants
@cakey

cakey commented Feb 22, 2018

A higher performance bulk next method. +tests +benchmarks

From issue #123

Sorry this has taken such a long time to put together.

Starting with the Bulk next functionality as hopefully it is less contentious.

In the end I went with a separate iterator as I was struggling to make the code compatible with a mixed next/nextMany mode (the existing iterator essentially prefetches the next value).

Also let me know areas you feel are missing tests/edge cases.

Anyway, the fun part! Benchmarks!

go test -bench BenchmarkNexts -benchmem -run -; and env BENCH_REAL_DATA=1 go test -bench BenchmarkRealDataNext -benchmem -run -
goos: darwin
goarch: amd64
pkg: git.soma.salesforce.com/WaveDataLib/vendor/github.com/RoaringBitmap/roaring
BenchmarkNexts/next__100.000000%-8         	     200	   9521314 ns/op	     288 B/op	       9 allocs/op
BenchmarkNexts/nextmany__100.000000%-8     	    1000	   1372220 ns/op	   16688 B/op	      10 allocs/op
BenchmarkNexts/next__50.000000%-8          	     200	   7505332 ns/op	     304 B/op	      17 allocs/op
BenchmarkNexts/nextmany__50.000000%-8      	    1000	   2268082 ns/op	   16944 B/op	      18 allocs/op
BenchmarkNexts/next__25.000000%-8          	     200	   7994177 ns/op	     544 B/op	      32 allocs/op
BenchmarkNexts/nextmany__25.000000%-8      	    1000	   2193432 ns/op	   17424 B/op	      33 allocs/op
BenchmarkNexts/next__12.500000%-8          	     200	   7530191 ns/op	    1056 B/op	      63 allocs/op
BenchmarkNexts/nextmany__12.500000%-8      	    1000	   2165133 ns/op	   18416 B/op	      64 allocs/op
BenchmarkNexts/next__6.250000%-8           	     300	   4945158 ns/op	    3984 B/op	     124 allocs/op
BenchmarkNexts/nextmany__6.250000%-8       	    1000	   1440372 ns/op	   20368 B/op	     125 allocs/op
BenchmarkNexts/next__3.125000%-8           	     300	   4958086 ns/op	    7888 B/op	     246 allocs/op
BenchmarkNexts/nextmany__3.125000%-8       	    1000	   1438189 ns/op	   24272 B/op	     247 allocs/op
BenchmarkNexts/next__1.562500%-8           	     300	   4776907 ns/op	   15696 B/op	     490 allocs/op
BenchmarkNexts/nextmany__1.562500%-8       	    1000	   1438678 ns/op	   32080 B/op	     491 allocs/op
BenchmarkNexts/next__0.390625%-8           	     300	   4920443 ns/op	   62576 B/op	    1955 allocs/op
BenchmarkNexts/nextmany__0.390625%-8       	    1000	   1464602 ns/op	   78960 B/op	    1956 allocs/op
BenchmarkNexts/next__0.097656%-8           	     300	   5364751 ns/op	  250069 B/op	    7814 allocs/op
BenchmarkNexts/nextmany__0.097656%-8       	    1000	   1877289 ns/op	  266448 B/op	    7815 allocs/op
BenchmarkNexts/next__0.012352%-8           	     200	   7851239 ns/op	 1976632 B/op	   61769 allocs/op
BenchmarkNexts/nextmany__0.012352%-8       	     300	   4957780 ns/op	 1993008 B/op	   61770 allocs/op
BenchmarkNextsRLE/next-8                   	     100	  20446490 ns/op	     560 B/op	      17 allocs/op
BenchmarkNextsRLE/nextmany-8               	     500	   2441484 ns/op	    8752 B/op	      18 allocs/op
PASS
ok  	git.soma.salesforce.com/WaveDataLib/vendor/github.com/RoaringBitmap/roaring	45.162s
goos: darwin
goarch: amd64
pkg: git.soma.salesforce.com/WaveDataLib/vendor/github.com/RoaringBitmap/roaring
BenchmarkRealDataNext/census-income_srt-8         	      10	 126063551 ns/op	   31664 B/op	     891 allocs/op
BenchmarkRealDataNext/census-income-8             	      10	 103090878 ns/op	   31296 B/op	     968 allocs/op
BenchmarkRealDataNext/census1881_srt-8            	     100	  14180454 ns/op	   90816 B/op	    2738 allocs/op
BenchmarkRealDataNext/census1881-8                	     100	  10721427 ns/op	   56448 B/op	    1664 allocs/op
BenchmarkRealDataNext/dimension_003-8             	      20	  82011816 ns/op	 1277696 B/op	   32187 allocs/op
BenchmarkRealDataNext/dimension_008-8             	      20	  60217097 ns/op	  542528 B/op	   14334 allocs/op
BenchmarkRealDataNext/dimension_033-8             	      20	  80506341 ns/op	   64880 B/op	    1941 allocs/op
BenchmarkRealDataNext/uscensus2000-8              	   10000	    172225 ns/op	   80672 B/op	    2421 allocs/op
BenchmarkRealDataNext/weather_sept_85_srt-8       	       5	 313757812 ns/op	   78976 B/op	    2368 allocs/op
BenchmarkRealDataNext/weather_sept_85-8           	      10	 177623417 ns/op	   92016 B/op	    3056 allocs/op
BenchmarkRealDataNext/wikileaks-noquotes_srt-8    	     300	   5656957 ns/op	   60000 B/op	    1775 allocs/op
BenchmarkRealDataNext/wikileaks-noquotes-8        	     300	   5930431 ns/op	   70144 B/op	    2092 allocs/op
BenchmarkRealDataNextMany/census-income_srt-8     	     300	   5854518 ns/op	   48096 B/op	     892 allocs/op
BenchmarkRealDataNextMany/census-income-8         	     100	  17181030 ns/op	   50560 B/op	     969 allocs/op
BenchmarkRealDataNextMany/census1881_srt-8        	    2000	   1061879 ns/op	  107200 B/op	    2739 allocs/op
BenchmarkRealDataNextMany/census1881-8            	    1000	   1291917 ns/op	   72832 B/op	    1665 allocs/op
BenchmarkRealDataNextMany/dimension_003-8         	     200	   6272633 ns/op	 1294080 B/op	   32188 allocs/op
BenchmarkRealDataNextMany/dimension_008-8         	     500	   3566583 ns/op	  558912 B/op	   14335 allocs/op
BenchmarkRealDataNextMany/dimension_033-8         	     500	   3721705 ns/op	   81264 B/op	    1942 allocs/op
BenchmarkRealDataNextMany/uscensus2000-8          	   10000	    140842 ns/op	   97056 B/op	    2422 allocs/op
BenchmarkRealDataNextMany/weather_sept_85_srt-8   	     100	  15765233 ns/op	   95360 B/op	    2369 allocs/op
BenchmarkRealDataNextMany/weather_sept_85-8       	      50	  33426390 ns/op	  117376 B/op	    3057 allocs/op
BenchmarkRealDataNextMany/wikileaks-noquotes_srt-8          3000	    468985 ns/op	   76384 B/op	    1776 allocs/op
BenchmarkRealDataNextMany/wikileaks-noquotes-8              2000	   1025329 ns/op	   86528 B/op	    2093 allocs/op
PASS
ok  	git.soma.salesforce.com/WaveDataLib/vendor/github.com/RoaringBitmap/roaring	84.778s

Summary of synthetic benchmarks

Density	 	 Speedup
100.000000% 	 -85.6%
50.000000% 	 -69.8%
25.000000% 	 -72.6%
12.500000% 	 -71.2%
6.250000% 	 -70.9%
3.125000% 	 -71.0%
1.562500% 	 -69.9%
0.390625% 	 -70.2%
0.097656% 	 -65.0%
0.012352% 	 -36.9%
RLE 	 	 -88.1%

3-4x faster
Drop in improvement is due to more iterator allocation overhead from sparse containers

Summary of Real-world benchmarks

Test	 	 Speedup
census-income_srt-8         	-95.4%
census-income-8             	-83.3%
census1881_srt-8            	-92.5%
census1881-8                	-88.0%
dimension_003-8             	-92.4%
dimension_008-8             	-94.1%
dimension_033-8             	-95.4%
uscensus2000-8              	-18.2%
weather_sept_85_srt-8       	-95.0%
weather_sept_85-8           	-81.2%
wikileaks-noquotes_srt-8    	-91.7%
wikileaks-noquotes-8        	-82.7%

5-20x faster

and func(bitmaps ... *Bitmap) *Bitmap,
or func(bitmaps ... *Bitmap) *Bitmap,
xor func(bitmaps ... *Bitmap) *Bitmap) {
and func(bitmaps ...*Bitmap) *Bitmap,

This comment has been minimized.

@cakey

cakey Feb 22, 2018

unrelated go fmt quirks

func (bcmi *bitmapContainerManyIterator) nextMany(hs uint32, buf []uint32) int {
n := 0
base := bcmi.base

This comment has been minimized.

@cakey

cakey Feb 22, 2018

go didn't seem to be able to optimise these away in the inner loop, but perhaps i was missing something (similar idiom in the other functions)

This comment has been minimized.

@maciej

maciej Feb 24, 2018

Member

The likely reason is that your referring to an pointer (*bcmi). The value that your referring might change anytime as it’s visible to other threads / goriutines.

@lemire

This comment has been minimized.

Member

lemire commented Feb 23, 2018

This looks good to me and the performance is great, according to your tests.

Maybe someone else should also review this.

@maciej what do you think?

@maciej

This comment has been minimized.

Member

maciej commented Feb 23, 2018

Looks good at first sight. I'll have a closer look within the next 24h, likely later today (European time).

@@ -251,6 +251,53 @@ func newIntIterator(a *Bitmap) *intIterator {
return p
}
// ManyIntIterable allows you to iterate over the values in a Bitmap
type ManyIntIterable interface {

This comment has been minimized.

@maciej

maciej Feb 24, 2018

Member

The /Int/ in in the name makes me a bit uneasy. /ManyUint32Iterator/? What do you think? Seems quite verbose in the other hand.

This comment has been minimized.

@cakey

cakey Feb 25, 2018

Yeah probably not ideal, but consistent with the IntIterable returned from Iterator()

@maciej

This comment has been minimized.

Member

maciej commented Feb 24, 2018

Looks good to me!

@lemire

This comment has been minimized.

Member

lemire commented Feb 26, 2018

So, let us merge!

@lemire lemire merged commit a530db3 into RoaringBitmap:master Feb 26, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

maciej pushed a commit to maciej/roaring that referenced this pull request Apr 10, 2018

Merge pull request RoaringBitmap#150 from cakey/ben-nextmany
Adds bitmap.NextMany() functionality
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment