Skip to content

internal/keyspan: apply various micro-optimizations#1743

Merged
jbowens merged 4 commits intocockroachdb:masterfrom
jbowens:opt-noop
Jun 9, 2022
Merged

internal/keyspan: apply various micro-optimizations#1743
jbowens merged 4 commits intocockroachdb:masterfrom
jbowens:opt-noop

Conversation

@jbowens
Copy link
Copy Markdown
Contributor

@jbowens jbowens commented Jun 8, 2022

These commits introduce a new microbenchmark BenchmarkIteratorSeekNoRangeKeys
and apply a few micro-optimizations that help move the needle on cockroachdb/cockroach#82559.

name                                    old time/op    new time/op    delta
IteratorSeekNoRangeKeys/batch=false-10    5.09µs ± 1%    4.28µs ± 1%  -16.03%  (p=0.000 n=10+19)
IteratorSeekNoRangeKeys/batch=true-10     9.85µs ± 1%    8.78µs ± 1%  -10.87%  (p=0.000 n=9+20)

@jbowens jbowens requested review from a team, itsbilal and nicktrav June 8, 2022 15:07
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Contributor

@nicktrav nicktrav left a comment

Choose a reason for hiding this comment

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

Fabulous! :lgtm:

Reviewed 1 of 1 files at r1, 1 of 1 files at r2, 1 of 1 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @itsbilal)

jbowens added 4 commits June 9, 2022 12:15
Previously, DefragmentingIter checked for invalid spans within
defragment{Forward,Backward}. This incurred unnecessary validity checks in some
cases, and in others forced a function call that could've been replaced by an
inlined validity check. This commit moves these validity checks into the
caller, where necessary.

```
name                                    old time/op    new time/op    delta
IteratorSeekNoRangeKeys/batch=false-10    5.09µs ± 1%    4.53µs ± 1%  -10.94%  (p=0.000 n=10+9)
IteratorSeekNoRangeKeys/batch=true-10     9.85µs ± 1%    9.54µs ± 3%   -3.15%  (p=0.000 n=9+10)
```
Inline the startBound and endBound helpers used in MergingIter. This also
removes a redundant span validity check in some cases.

```
name                                    old time/op    new time/op    delta
IteratorSeekNoRangeKeys/batch=false-10    4.57µs ± 1%    4.59µs ± 1%    ~     (p=0.164 n=19+20)
IteratorSeekNoRangeKeys/batch=true-10     9.53µs ± 2%    9.04µs ± 4%  -5.11%  (p=0.000 n=19+18)
```
Refactor the InterleavingIter's bound checking to remove unnecessary Span
struct copies as parameters and return values.

```
name                                    old time/op    new time/op    delta
IteratorSeekNoRangeKeys/batch=false-10    4.59µs ± 1%    4.28µs ± 1%  -6.79%  (p=0.000 n=20+19)
IteratorSeekNoRangeKeys/batch=true-10     9.04µs ± 4%    8.78µs ± 1%  -2.94%  (p=0.000 n=18+20)
```
Copy link
Copy Markdown
Contributor Author

@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.

TFTR!

Reviewable status: 1 of 4 files reviewed, all discussions resolved (waiting on @itsbilal and @nicktrav)

@jbowens jbowens merged commit 83b45b0 into cockroachdb:master Jun 9, 2022
@jbowens jbowens deleted the opt-noop branch June 9, 2022 16:41
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.

3 participants