Skip to content

storage: load pebbleMVCCScanner position when enabling point synthesis#85710

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
erikgrinaker:point-synthesis-init
Aug 9, 2022
Merged

storage: load pebbleMVCCScanner position when enabling point synthesis#85710
craig[bot] merged 1 commit intocockroachdb:masterfrom
erikgrinaker:point-synthesis-init

Conversation

@erikgrinaker
Copy link
Copy Markdown
Contributor

@erikgrinaker erikgrinaker commented Aug 7, 2022

This patch loads the current iterator position when switching to point
synthesis in pebbleMVCCScanner, rather than initializing the point
synthesizing iterator using an additional seek.

An attempt was also made at pooling the pointSynthesizingIter together
with the pebbleMVCCScanner instead of using a separate pool, but this
didn't appear to have a significant effect on performance.

name                                                                   old time/op    new time/op    delta
MVCCGet_Pebble/batch=true/versions=1/valueSize=8/numRangeKeys=0-24       2.74µs ± 1%    2.76µs ± 1%   +0.84%  (p=0.013 n=10+10)
MVCCGet_Pebble/batch=true/versions=1/valueSize=8/numRangeKeys=1-24       5.40µs ± 2%    3.57µs ± 1%  -33.86%  (p=0.000 n=10+10)
MVCCGet_Pebble/batch=true/versions=1/valueSize=8/numRangeKeys=100-24      171µs ± 2%     166µs ± 1%   -2.40%  (p=0.000 n=9+10)
MVCCGet_Pebble/batch=true/versions=10/valueSize=8/numRangeKeys=0-24      3.90µs ± 0%    3.90µs ± 1%     ~     (p=0.968 n=9+10)
MVCCGet_Pebble/batch=true/versions=10/valueSize=8/numRangeKeys=1-24      7.11µs ± 1%    5.13µs ± 1%  -27.87%  (p=0.000 n=10+10)
MVCCGet_Pebble/batch=true/versions=10/valueSize=8/numRangeKeys=100-24     179µs ± 1%     174µs ± 1%   -2.52%  (p=0.000 n=10+10)

Resolves #84380.

Release note: None

@erikgrinaker erikgrinaker requested review from a team and jbowens August 7, 2022 13:45
@erikgrinaker erikgrinaker self-assigned this Aug 7, 2022
@erikgrinaker erikgrinaker requested a review from a team as a code owner August 7, 2022 13:45
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@erikgrinaker erikgrinaker changed the title storage: load `pebbleMVCCScanner´ position when enabling point synthesis storage: load pebbleMVCCScanner position when enabling point synthesis Aug 7, 2022
@erikgrinaker erikgrinaker force-pushed the point-synthesis-init branch 2 times, most recently from a6c261c to 37b24a7 Compare August 7, 2022 13:56
Copy link
Copy Markdown
Contributor

@jbowens jbowens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

🚀

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @erikgrinaker)

@erikgrinaker erikgrinaker force-pushed the point-synthesis-init branch from 37b24a7 to b8d90ce Compare August 8, 2022 19:58
This patch loads the current iterator position when switching to point
synthesis in `pebbleMVCCScanner`, rather than initializing the point
synthesizing iterator using an additional seek.

An attempt was also made at pooling the `pointSynthesizingIter` together
with the `pebbleMVCCScanner` instead of using a separate pool, but this
didn't appear to have a significant effect on performance.

```
name                                                                   old time/op    new time/op    delta
MVCCGet_Pebble/batch=true/versions=1/valueSize=8/numRangeKeys=0-24       2.74µs ± 1%    2.76µs ± 1%   +0.84%  (p=0.013 n=10+10)
MVCCGet_Pebble/batch=true/versions=1/valueSize=8/numRangeKeys=1-24       5.40µs ± 2%    3.57µs ± 1%  -33.86%  (p=0.000 n=10+10)
MVCCGet_Pebble/batch=true/versions=1/valueSize=8/numRangeKeys=100-24      171µs ± 2%     166µs ± 1%   -2.40%  (p=0.000 n=9+10)
MVCCGet_Pebble/batch=true/versions=10/valueSize=8/numRangeKeys=0-24      3.90µs ± 0%    3.90µs ± 1%     ~     (p=0.968 n=9+10)
MVCCGet_Pebble/batch=true/versions=10/valueSize=8/numRangeKeys=1-24      7.11µs ± 1%    5.13µs ± 1%  -27.87%  (p=0.000 n=10+10)
MVCCGet_Pebble/batch=true/versions=10/valueSize=8/numRangeKeys=100-24     179µs ± 1%     174µs ± 1%   -2.52%  (p=0.000 n=10+10)
```

Release note: None
@erikgrinaker erikgrinaker force-pushed the point-synthesis-init branch from b8d90ce to ce222fe Compare August 8, 2022 21:35
@erikgrinaker
Copy link
Copy Markdown
Contributor Author

bors r=jbowens

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 9, 2022

Build succeeded:

@craig craig bot merged commit 3bc5b49 into cockroachdb:master Aug 9, 2022
@erikgrinaker erikgrinaker deleted the point-synthesis-init branch August 12, 2022 12:43
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.

storage: optimize pointSynthesizingIter initialization

3 participants