Skip to content
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

Add FromUnsafeBytes to prevent small allocations caused by ByteInputAdapter #395

Merged
merged 3 commits into from Aug 17, 2023

Conversation

damnever
Copy link
Contributor

We are utilizing the rocksdb merge operator to manipulate values that are encoded by roaring bitmap. The bitmap will be read from disk and merged together, these frequent operations can consume significant CPU resources due to the memory allocations.

So, I am proposing to add an alternative function called FromUnsafeBytes to address this problem. The caller can decide whether to make a copy or reuse those byte buffers if necessary.

The benchmark results (note that I have allocated a large buffer and made a copy before FromUnsafeBytes)

$ gotest -v -benchmem -bench=.+ -run=Benchmark -benchtime=100000x -count=1
goos: darwin
goarch: arm64
pkg: github.com/RoaringBitmap/roaring/roaring64
BenchmarkUnserializeFromUnsafeBytes
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-650
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-650-10                100000               403.2 ns/op          1660 B/op         11 allocs/op
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-6500
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-6500-10               100000               886.9 ns/op          9732 B/op         11 allocs/op
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-65000
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-65000-10              100000               839.7 ns/op          9732 B/op         11 allocs/op
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-650000
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-650000-10             100000              1106 ns/op           10120 B/op         20 allocs/op
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-6500000
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-6500000-10            100000              3643 ns/op           14456 B/op        110 allocs/op
BenchmarkUnserializeReadFrom
BenchmarkUnserializeReadFrom/ReadFrom-650
BenchmarkUnserializeReadFrom/ReadFrom-650-10                              100000               459.3 ns/op          1712 B/op         19 allocs/op
BenchmarkUnserializeReadFrom/ReadFrom-6500
BenchmarkUnserializeReadFrom/ReadFrom-6500-10                             100000              1074 ns/op            8507 B/op         19 allocs/op
BenchmarkUnserializeReadFrom/ReadFrom-65000
BenchmarkUnserializeReadFrom/ReadFrom-65000-10                            100000              1109 ns/op            8507 B/op         19 allocs/op
BenchmarkUnserializeReadFrom/ReadFrom-650000
BenchmarkUnserializeReadFrom/ReadFrom-650000-10                           100000              1705 ns/op            9059 B/op         46 allocs/op
BenchmarkUnserializeReadFrom/ReadFrom-6500000
BenchmarkUnserializeReadFrom/ReadFrom-6500000-10                          100000              6575 ns/op           14605 B/op        316 allocs/op
PASS

@damnever
Copy link
Contributor Author

Some irrelevant changes are caused by the auto-formatter of my editor..

…dapter

Signed-off-by: Xiaochao Dong (@damnever) <the.xcdong@gmail.com>
@damnever
Copy link
Contributor Author

damnever commented Aug 16, 2023

Updated benchmark result with damnever@e8266c1:

$ gotest -v -benchmem -bench=Benchmark -run=Benchmark -benchtime=1000000x
goos: darwin
goarch: arm64
pkg: github.com/RoaringBitmap/roaring/roaring64
BenchmarkUnserializeFromUnsafeBytes
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-650
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-650-10               1000000               342.2 ns/op          1660 B/op         11 allocs/op
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-6500
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-6500-10              1000000               863.1 ns/op          9732 B/op         11 allocs/op
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-65000
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-65000-10             1000000               853.6 ns/op          9732 B/op         11 allocs/op
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-650000
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-650000-10            1000000              1133 ns/op           10120 B/op         20 allocs/op
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-6500000
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-6500000-10           1000000              3678 ns/op           14456 B/op        110 allocs/op
BenchmarkUnserializeReadFrom
BenchmarkUnserializeReadFrom/ReadFrom-650
BenchmarkUnserializeReadFrom/ReadFrom-650-10                             1000000               451.4 ns/op          1704 B/op         17 allocs/op
BenchmarkUnserializeReadFrom/ReadFrom-6500
BenchmarkUnserializeReadFrom/ReadFrom-6500-10                            1000000              1117 ns/op            8499 B/op         17 allocs/op
BenchmarkUnserializeReadFrom/ReadFrom-65000
BenchmarkUnserializeReadFrom/ReadFrom-65000-10                           1000000              1087 ns/op            8499 B/op         17 allocs/op
BenchmarkUnserializeReadFrom/ReadFrom-650000
BenchmarkUnserializeReadFrom/ReadFrom-650000-10                          1000000              1755 ns/op            9019 B/op         36 allocs/op
BenchmarkUnserializeReadFrom/ReadFrom-6500000
BenchmarkUnserializeReadFrom/ReadFrom-6500000-10                         1000000              6223 ns/op           14205 B/op        216 allocs/op
PASS

Signed-off-by: Xiaochao Dong (@damnever) <the.xcdong@gmail.com>
Signed-off-by: Xiaochao Dong (@damnever) <the.xcdong@gmail.com>
@damnever
Copy link
Contributor Author

damnever commented Aug 16, 2023

We can now reuse the underlying slices of Bitmap after Clear, and reuse the sizeBuf as KeyBuf to avoid allocation damnever@ba2dbcb

$ gotest -v -benchmem -bench=Benchmark -run=Benchmark -benchtime=1000000x
goos: darwin
goarch: arm64
pkg: github.com/RoaringBitmap/roaring/roaring64
BenchmarkUnserializeFromUnsafeBytes
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-650
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-650-10               1000000               345.2 ns/op          1660 B/op         11 allocs/op
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-6500
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-6500-10              1000000               880.4 ns/op          9732 B/op         11 allocs/op
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-65000
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-65000-10             1000000               865.7 ns/op          9732 B/op         11 allocs/op
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-650000
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-650000-10            1000000              1135 ns/op           10120 B/op         20 allocs/op
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-6500000
BenchmarkUnserializeFromUnsafeBytes/FromUnsafeBytes-6500000-10           1000000              3669 ns/op           14456 B/op        110 allocs/op
BenchmarkUnserializeReadFrom
BenchmarkUnserializeReadFrom/ReadFrom-650
BenchmarkUnserializeReadFrom/ReadFrom-650-10                             1000000               443.1 ns/op          1704 B/op         16 allocs/op
BenchmarkUnserializeReadFrom/ReadFrom-6500
BenchmarkUnserializeReadFrom/ReadFrom-6500-10                            1000000              1110 ns/op            8499 B/op         16 allocs/op
BenchmarkUnserializeReadFrom/ReadFrom-65000
BenchmarkUnserializeReadFrom/ReadFrom-65000-10                           1000000              1155 ns/op            8499 B/op         16 allocs/op
BenchmarkUnserializeReadFrom/ReadFrom-650000
BenchmarkUnserializeReadFrom/ReadFrom-650000-10                          1000000              1639 ns/op            9019 B/op         35 allocs/op
BenchmarkUnserializeReadFrom/ReadFrom-6500000
BenchmarkUnserializeReadFrom/ReadFrom-6500000-10                         1000000              6130 ns/op           14205 B/op        215 allocs/op
PASS

@lemire
Copy link
Member

lemire commented Aug 16, 2023

Running tests.

@lemire
Copy link
Member

lemire commented Aug 16, 2023

@damnever If you recommend merging this, then I will do so and issue a release.

It seems uncontroversial.

@damnever
Copy link
Contributor Author

@lemire Yes, I would say it is safe to merge this since I have experimentally used it in our system.

@lemire lemire merged commit 952b765 into RoaringBitmap:master Aug 17, 2023
5 checks passed
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.

None yet

2 participants