Skip to content

util/interval: fix BenchmarkBTreeDeleteInsertCloneEachTime#62294

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
nvb:nvanbenschoten/intervalBtreeBench
Mar 26, 2021
Merged

util/interval: fix BenchmarkBTreeDeleteInsertCloneEachTime#62294
craig[bot] merged 1 commit intocockroachdb:masterfrom
nvb:nvanbenschoten/intervalBtreeBench

Conversation

@nvb
Copy link
Copy Markdown
Contributor

@nvb nvb commented Mar 20, 2021

The benchmark was not using the subtest's b.N, so it was getting bogus
results. This confused #62212, which saw a regression from 0.000008611111111
nanoseconds to 0.0000101 nanoseconds.

With this fix, the benchmark no longer shows any movement over the past release
and produces much more realistic results:

cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
BenchmarkBTreeDeleteInsertCloneEachTime/count=16/reset=false-16         	 2367748	       492.7 ns/op
BenchmarkBTreeDeleteInsertCloneEachTime/count=16/reset=true-16          	 6090852	       191.7 ns/op
BenchmarkBTreeDeleteInsertCloneEachTime/count=128/reset=false-16        	  991534	      1102 ns/op
BenchmarkBTreeDeleteInsertCloneEachTime/count=128/reset=true-16         	 2328866	       508.7 ns/op
BenchmarkBTreeDeleteInsertCloneEachTime/count=1024/reset=false-16       	  550702	      2088 ns/op
BenchmarkBTreeDeleteInsertCloneEachTime/count=1024/reset=true-16        	 1209684	      1054 ns/op
BenchmarkBTreeDeleteInsertCloneEachTime/count=8192/reset=false-16       	  463558	      2176 ns/op
BenchmarkBTreeDeleteInsertCloneEachTime/count=8192/reset=true-16        	  724923	      1542 ns/op
BenchmarkBTreeDeleteInsertCloneEachTime/count=65536/reset=false-16      	  264100	      4833 ns/op
BenchmarkBTreeDeleteInsertCloneEachTime/count=65536/reset=true-16       	  429841	      2467 ns/op

@nvb
util/interval: fix BenchmarkBTreeDeleteInsertCloneEachTime
3a6b152 
The benchmark was not using the subtest's `b.N`, so it was getting bogus
results. This confused cockroachdb#62212, which saw a regression from 0.000008611111111
nanoseconds to 0.0000101 nanoseconds.

With this fix, the benchmark no longer shows any movement over the past release.
@cockroach-teamcity
Copy link
Copy Markdown
Member

cockroach-teamcity commented Mar 20, 2021

This change is Reviewable

@nvb nvb mentioned this pull request Mar 20, 2021
Closed
35 tasks
@nvb nvb requested a review from aayushshah15 March 23, 2021 14:08
aayushshah15
aayushshah15 approved these changes Mar 25, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@aayushshah15 aayushshah15 left a comment

Choose a reason for hiding this comment

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

LGTM

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

@nvb
Copy link
Copy Markdown
Contributor Author

nvb commented Mar 25, 2021

TFTR!

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Mar 26, 2021

Build succeeded:

@craig craig bot merged commit 56ef155 into cockroachdb:master Mar 26, 2021
@nvb nvb deleted the nvanbenschoten/intervalBtreeBench branch March 29, 2021 22:05
@cucaroach cucaroach mentioned this pull request Oct 28, 2021
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@aayushshah15 aayushshah15 aayushshah15 approved these changes

Reviewers whose approvals may not affect merge requirements

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