Skip to content

kv: avoid retry error allocation on each Txn.exec call#74350

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
nvb:nvanbenschoten/assortedPerf14
Jan 3, 2022
Merged

kv: avoid retry error allocation on each Txn.exec call#74350
craig[bot] merged 1 commit intocockroachdb:masterfrom
nvb:nvanbenschoten/assortedPerf14

Conversation

@nvb
Copy link
Copy Markdown
Contributor

@nvb nvb commented Dec 30, 2021

Similar to #74339.

This commit updates Txn.exec to only call errors.As if the error from the
transaction execution was non-nil. This avoids a heap allocation of a
TransactionRetryWithProtoRefreshError on each call to Txn.exec, which was
originally introduced in a8ae1bf.

In kv100 (100% reads), this is responsible for 0.36% of total CPU usage.

Screen Shot 2021-12-30 at 6 16 57 PM

This is part of a collection of assorted micro-optimizations:

Combined, these changes have the following effect on end-to-end SQL query performance:

name                      old time/op    new time/op    delta
KV/Scan/SQL/rows=1-10       94.4µs ±10%    92.3µs ±11%   -2.20%  (p=0.000 n=93+93)
KV/Scan/SQL/rows=10-10       102µs ±10%      99µs ±10%   -2.16%  (p=0.000 n=94+94)
KV/Update/SQL/rows=10-10     378µs ±15%     370µs ±11%   -2.04%  (p=0.003 n=95+91)
KV/Insert/SQL/rows=1-10      133µs ±14%     132µs ±12%     ~     (p=0.738 n=95+93)
KV/Insert/SQL/rows=10-10     197µs ±14%     196µs ±13%     ~     (p=0.902 n=95+94)
KV/Update/SQL/rows=1-10      186µs ±14%     185µs ±14%     ~     (p=0.351 n=94+93)
KV/Delete/SQL/rows=1-10      132µs ±13%     132µs ±14%     ~     (p=0.473 n=94+94)
KV/Delete/SQL/rows=10-10     254µs ±16%     250µs ±16%     ~     (p=0.086 n=100+99)

name                      old alloc/op   new alloc/op   delta
KV/Scan/SQL/rows=1-10       20.1kB ± 0%    19.1kB ± 1%   -4.91%  (p=0.000 n=96+96)
KV/Scan/SQL/rows=10-10      21.7kB ± 0%    20.7kB ± 1%   -4.61%  (p=0.000 n=96+97)
KV/Delete/SQL/rows=10-10    64.0kB ± 3%    63.7kB ± 3%   -0.55%  (p=0.000 n=100+100)
KV/Update/SQL/rows=1-10     45.8kB ± 1%    45.5kB ± 1%   -0.55%  (p=0.000 n=97+98)
KV/Update/SQL/rows=10-10     105kB ± 1%     105kB ± 1%   -0.10%  (p=0.008 n=97+98)
KV/Delete/SQL/rows=1-10     40.8kB ± 0%    40.7kB ± 0%   -0.08%  (p=0.001 n=95+96)
KV/Insert/SQL/rows=1-10     37.4kB ± 1%    37.4kB ± 0%     ~     (p=0.698 n=97+96)
KV/Insert/SQL/rows=10-10    76.4kB ± 1%    76.4kB ± 0%     ~     (p=0.822 n=99+98)

name                      old allocs/op  new allocs/op  delta
KV/Scan/SQL/rows=1-10          245 ± 0%       217 ± 0%  -11.43%  (p=0.000 n=95+92)
KV/Scan/SQL/rows=10-10         280 ± 0%       252 ± 0%  -10.11%  (p=0.000 n=75+97)
KV/Delete/SQL/rows=10-10       478 ± 0%       459 ± 0%   -4.04%  (p=0.000 n=94+97)
KV/Delete/SQL/rows=1-10        297 ± 1%       287 ± 1%   -3.34%  (p=0.000 n=97+97)
KV/Update/SQL/rows=1-10        459 ± 0%       444 ± 0%   -3.27%  (p=0.000 n=97+97)
KV/Insert/SQL/rows=1-10        291 ± 0%       286 ± 0%   -1.72%  (p=0.000 n=82+86)
KV/Update/SQL/rows=10-10       763 ± 1%       750 ± 1%   -1.68%  (p=0.000 n=96+98)
KV/Insert/SQL/rows=10-10       489 ± 0%       484 ± 0%   -1.03%  (p=0.000 n=98+98)

@nvb
kv: avoid retry error allocation on each Txn.exec call
1f021e4 
This commit updates `Txn.exec` to only call `errors.As` if the error from the
transaction execution was non-nil. This avoids a heap allocation of a
`TransactionRetryWithProtoRefreshError` on each call to `Txn.exec`, which was
originally introduced in a8ae1bf.

In kv100 (100% reads), this is responsible for **0.36%** of total CPU usage.
@nvb nvb requested a review from knz December 30, 2021 23:18
@nvb nvb requested a review from a team as a code owner December 30, 2021 23:18
@cockroach-teamcity
Copy link
Copy Markdown
Member

cockroach-teamcity commented Dec 30, 2021

This change is Reviewable

@nvb nvb mentioned this pull request Dec 30, 2021
Merged
tbg
tbg approved these changes Jan 3, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@tbg tbg left a comment

Choose a reason for hiding this comment

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

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

@nvb
Copy link
Copy Markdown
Contributor Author

nvb commented Jan 3, 2022

TFTR!

bors r=tbg

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jan 3, 2022

🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

@craig craig bot merged commit 5669b9d into cockroachdb:master Jan 3, 2022
@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jan 3, 2022

Build succeeded:

@nvb nvb deleted the nvanbenschoten/assortedPerf14 branch January 6, 2022 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tbg tbg tbg approved these changes

@knz knz Awaiting requested review from knz

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nvb @cockroach-teamcity @tbg