Skip to content

manifest: avoid BlobReferences interface allocation #4678

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jbowens
Copy link
Collaborator

@jbowens jbowens commented May 2, 2025

db: add BenchmarkPointLookupSeparatedValues

Add a microbenchmark that exercises a point lookup wihtin a database with
separated values.

Informs #112.

manifest: avoid BlobReferences interface allocation

The manifest.BlobReferences type is used to satisfy the sstable.BlobReferences
interface. Previously the interface was implemented on the slice receiver type.
This caused an allocation whenever we used this type to satisfy the
sstable.BlobReferences interface. This commit moves the implementation onto the
pointer type, avoiding the need to heap-allocate a copy of the slice header.

goos: darwin
goarch: arm64
pkg: github.com/cockroachdb/pebble
cpu: Apple M1 Pro
                                                    │ before.txt  │              after.txt               │
                                                    │   sec/op    │   sec/op     vs base                 │
PointLookupSeparatedValues/keys=1m,valueLen=100-10    1.636µ ± 8%   1.585µ ± 2%  -3.09% (p=0.001 n=10)
PointLookupSeparatedValues/keys=1m,valueLen=1024-10   2.452µ ± 4%   2.373µ ± 4%  -3.22% (p=0.007 n=10+6)
geomean                                               2.003µ        1.939µ       -3.15%

                                                    │ before.txt  │               after.txt               │
                                                    │    B/op     │    B/op      vs base                  │
PointLookupSeparatedValues/keys=1m,valueLen=100-10    32.000 ± 0%   8.000 ±  0%  -75.00% (p=0.000 n=10)
PointLookupSeparatedValues/keys=1m,valueLen=1024-10    265.5 ± 7%   210.0 ± 22%  -20.90% (p=0.001 n=10+6)
geomean                                                92.17        40.99        -55.53%

                                                    │ before.txt │                after.txt                │
                                                    │ allocs/op  │ allocs/op   vs base                     │
PointLookupSeparatedValues/keys=1m,valueLen=100-10    1.000 ± 0%   0.000 ± 0%  -100.00% (p=0.000 n=10)
PointLookupSeparatedValues/keys=1m,valueLen=1024-10   1.000 ± 0%   0.000 ± 0%  -100.00% (n=10+6)
geomean                                               1.000                    ?                       ¹ ²
¹ summaries must be >0 to compute geomean
² ratios must be >0 to compute geomean

Fix #4668.

jbowens added 2 commits May 1, 2025 23:35
Add a microbenchmark that exercises a point lookup wihtin a database with
separated values.

Informs cockroachdb#112.
The manifest.BlobReferences type is used to satisfy the sstable.BlobReferences
interface. Previously the interface was implemented on the slice receiver type.
This caused an allocation whenever we used this type to satisfy the
sstable.BlobReferences interface. This commit moves the implementation onto the
pointer type, avoiding the need to heap-allocate a copy of the slice header.

```
goos: darwin
goarch: arm64
pkg: github.com/cockroachdb/pebble
cpu: Apple M1 Pro
                                                    │ before.txt  │              after.txt               │
                                                    │   sec/op    │   sec/op     vs base                 │
PointLookupSeparatedValues/keys=1m,valueLen=100-10    1.636µ ± 8%   1.585µ ± 2%  -3.09% (p=0.001 n=10)
PointLookupSeparatedValues/keys=1m,valueLen=1024-10   2.452µ ± 4%   2.373µ ± 4%  -3.22% (p=0.007 n=10+6)
geomean                                               2.003µ        1.939µ       -3.15%

                                                    │ before.txt  │               after.txt               │
                                                    │    B/op     │    B/op      vs base                  │
PointLookupSeparatedValues/keys=1m,valueLen=100-10    32.000 ± 0%   8.000 ±  0%  -75.00% (p=0.000 n=10)
PointLookupSeparatedValues/keys=1m,valueLen=1024-10    265.5 ± 7%   210.0 ± 22%  -20.90% (p=0.001 n=10+6)
geomean                                                92.17        40.99        -55.53%

                                                    │ before.txt │                after.txt                │
                                                    │ allocs/op  │ allocs/op   vs base                     │
PointLookupSeparatedValues/keys=1m,valueLen=100-10    1.000 ± 0%   0.000 ± 0%  -100.00% (p=0.000 n=10)
PointLookupSeparatedValues/keys=1m,valueLen=1024-10   1.000 ± 0%   0.000 ± 0%  -100.00% (n=10+6)
geomean                                               1.000                    ?                       ¹ ²
¹ summaries must be >0 to compute geomean
² ratios must be >0 to compute geomean
```

Fix cockroachdb#4668.
@jbowens jbowens requested a review from a team as a code owner May 2, 2025 13:01
@jbowens jbowens requested a review from sumeerbhola May 2, 2025 13:01
@cockroach-teamcity
Copy link
Member

This change is Reviewable

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.

db: allocation creating a sstable iterator with blob refs
2 participants