sql: add merge joiner benchmark and bytes usage optimization #31216

changangela · Oct 10, 2018

Reverted #30924 for now to compare different approaches (discussed in #31191). This PR is mainly for adding some merge joiner benchmarks as well a small change in BoundAccount.Shrink() that significantly improves the merge joiner performance. This way, we can safely backport this change.

MergeJoinerBenchmark against release-2.1:

name                           old time/op    new time/op    delta
MergeJoiner/InputSize=0-8        4.31µs ±10%    4.33µs ±15%      ~     (p=0.968 n=10+9)
MergeJoiner/InputSize=4-8        8.34µs ± 3%    8.10µs ± 4%      ~     (p=0.074 n=8+9)
MergeJoiner/InputSize=16-8       16.3µs ± 2%    11.1µs ± 5%   -31.87%  (p=0.000 n=9+9)
MergeJoiner/InputSize=256-8       190µs ± 2%      85µs ± 2%   -55.04%  (p=0.000 n=10+8)
MergeJoiner/InputSize=4096-8     2.96ms ± 2%    1.28ms ± 2%   -56.86%  (p=0.000 n=10+10)
MergeJoiner/InputSize=65536-8    49.0ms ± 5%    20.6ms ± 1%   -57.88%  (p=0.000 n=10+10)

name                           old alloc/op   new alloc/op   delta
MergeJoiner/InputSize=0-8        6.42kB ± 0%    6.65kB ± 0%    +3.49%  (p=0.000 n=10+10)
MergeJoiner/InputSize=4-8        9.50kB ± 0%    9.72kB ± 0%    +2.36%  (p=0.000 n=10+10)
MergeJoiner/InputSize=16-8       9.50kB ± 0%    9.72kB ± 0%    +2.36%  (p=0.000 n=10+10)
MergeJoiner/InputSize=256-8      32.5kB ± 0%    32.8kB ± 0%    +0.69%  (p=0.000 n=10+10)
MergeJoiner/InputSize=4096-8      401kB ± 0%     401kB ± 0%    +0.06%  (p=0.000 n=9+10)
MergeJoiner/InputSize=65536-8    6.30MB ± 0%    6.30MB ± 0%    +0.00%  (p=0.000 n=9+10)

name                           old allocs/op  new allocs/op  delta
MergeJoiner/InputSize=0-8          14.0 ± 0%      15.0 ± 0%    +7.14%  (p=0.000 n=10+10)
MergeJoiner/InputSize=4-8          16.0 ± 0%      17.0 ± 0%    +6.25%  (p=0.000 n=10+10)
MergeJoiner/InputSize=16-8         16.0 ± 0%      17.0 ± 0%    +6.25%  (p=0.000 n=10+10)
MergeJoiner/InputSize=256-8        46.0 ± 0%      47.0 ± 0%    +2.17%  (p=0.000 n=10+10)
MergeJoiner/InputSize=4096-8        526 ± 0%       527 ± 0%    +0.19%  (p=0.000 n=10+10)
MergeJoiner/InputSize=65536-8     8.21k ± 0%     8.21k ± 0%    +0.01%  (p=0.000 n=10+10)

name                           old speed      new speed      delta
MergeJoiner/InputSize=4-8      7.67MB/s ± 3%  7.91MB/s ± 4%      ~     (p=0.070 n=8+9)
MergeJoiner/InputSize=16-8     15.7MB/s ± 2%  23.0MB/s ± 5%   +46.89%  (p=0.000 n=9+9)
MergeJoiner/InputSize=256-8    21.6MB/s ± 2%  48.0MB/s ± 2%  +122.41%  (p=0.000 n=10+8)
MergeJoiner/InputSize=4096-8   22.1MB/s ± 2%  51.3MB/s ± 2%  +131.81%  (p=0.000 n=10+10)
MergeJoiner/InputSize=65536-8  21.4MB/s ± 5%  50.8MB/s ± 1%  +137.16%  (p=0.000 n=10+10)

MergeJoinerBenchmark compared with master (master already has this exact optimization, we want to ensure that the performance did not deteriorate)

name                                          old time/op    new time/op    delta
MergeJoiner/InputSize=0-8                       4.61µs ± 9%    4.45µs ± 5%     ~     (p=0.060 n=10+10)
MergeJoiner/InputSize=4-8                       8.34µs ± 6%    8.00µs ±10%   -4.09%  (p=0.037 n=9+10)
MergeJoiner/InputSize=16-8                      11.6µs ± 4%    11.4µs ± 5%     ~     (p=0.123 n=10+10)
MergeJoiner/InputSize=256-8                     88.3µs ± 3%    89.8µs ± 6%     ~     (p=0.258 n=9+9)
MergeJoiner/InputSize=4096-8                    1.33ms ± 4%    1.27ms ± 5%   -4.67%  (p=0.001 n=9+10)
MergeJoiner/InputSize=65536-8                   22.4ms ± 6%    21.3ms ±10%     ~     (p=0.052 n=10+10)
MergeJoiner/OneSideRepeatInputSize=0-8          4.57µs ±15%    4.38µs ± 6%     ~     (p=0.353 n=10+10)
MergeJoiner/OneSideRepeatInputSize=4-8          7.71µs ±10%    7.71µs ± 4%     ~     (p=0.549 n=10+9)
MergeJoiner/OneSideRepeatInputSize=16-8         11.8µs ±29%    10.5µs ± 6%  -10.77%  (p=0.043 n=10+10)
MergeJoiner/OneSideRepeatInputSize=256-8        82.8µs ± 4%    80.3µs ± 5%   -2.93%  (p=0.004 n=10+10)
MergeJoiner/OneSideRepeatInputSize=4096-8       1.25ms ± 2%    1.54ms ±13%  +23.09%  (p=0.000 n=9+10)
MergeJoiner/OneSideRepeatInputSize=65536-8      24.2ms ± 3%    26.7ms ± 9%  +10.08%  (p=0.000 n=10+9)
MergeJoiner/BothSidesRepeatInputSize=0-8        4.60µs ±10%    4.36µs ±10%     ~     (p=0.063 n=10+10)
MergeJoiner/BothSidesRepeatInputSize=4-8        7.13µs ± 4%    7.62µs ±17%   +6.84%  (p=0.005 n=9+10)
MergeJoiner/BothSidesRepeatInputSize=16-8       8.66µs ±14%    8.24µs ± 3%     ~     (p=0.549 n=10+9)
MergeJoiner/BothSidesRepeatInputSize=256-8      22.1µs ± 5%    23.2µs ± 9%   +5.06%  (p=0.004 n=9+10)
MergeJoiner/BothSidesRepeatInputSize=4096-8      219µs ± 4%     240µs ±25%     ~     (p=0.065 n=9+10)
MergeJoiner/BothSidesRepeatInputSize=65536-8    1.19ms ± 3%    1.17ms ± 1%   -2.18%  (p=0.001 n=10+9)

name                                          old alloc/op   new alloc/op   delta
MergeJoiner/InputSize=0-8                       6.66kB ± 0%    6.65kB ± 0%   -0.24%  (p=0.000 n=10+10)
MergeJoiner/InputSize=4-8                       9.74kB ± 0%    9.72kB ± 0%   -0.16%  (p=0.000 n=10+10)
MergeJoiner/InputSize=16-8                      9.74kB ± 0%    9.72kB ± 0%   -0.16%  (p=0.000 n=10+10)
MergeJoiner/InputSize=256-8                     32.8kB ± 0%    32.8kB ± 0%   -0.05%  (p=0.000 n=10+10)
MergeJoiner/InputSize=4096-8                     401kB ± 0%     401kB ± 0%   -0.00%  (p=0.000 n=10+10)
MergeJoiner/InputSize=65536-8                   6.30MB ± 0%    6.30MB ± 0%   -0.00%  (p=0.000 n=10+10)
MergeJoiner/OneSideRepeatInputSize=0-8          6.66kB ± 0%    6.65kB ± 0%   -0.24%  (p=0.000 n=10+10)
MergeJoiner/OneSideRepeatInputSize=4-8          9.74kB ± 0%    9.72kB ± 0%   -0.16%  (p=0.000 n=10+10)
MergeJoiner/OneSideRepeatInputSize=16-8         9.74kB ± 0%    9.72kB ± 0%   -0.16%  (p=0.000 n=10+10)
MergeJoiner/OneSideRepeatInputSize=256-8        42.0kB ± 0%    42.0kB ± 0%   -0.04%  (p=0.000 n=10+10)
MergeJoiner/OneSideRepeatInputSize=4096-8        751kB ± 0%     751kB ± 0%   -0.00%  (p=0.000 n=10+10)
MergeJoiner/OneSideRepeatInputSize=65536-8      15.5MB ± 0%    15.5MB ± 0%   -0.00%  (p=0.000 n=10+10)
MergeJoiner/BothSidesRepeatInputSize=0-8        6.66kB ± 0%    6.65kB ± 0%   -0.24%  (p=0.000 n=10+10)
MergeJoiner/BothSidesRepeatInputSize=4-8        9.74kB ± 0%    9.72kB ± 0%   -0.16%  (p=0.000 n=10+10)
MergeJoiner/BothSidesRepeatInputSize=16-8       9.74kB ± 0%    9.72kB ± 0%   -0.16%  (p=0.000 n=10+10)
MergeJoiner/BothSidesRepeatInputSize=256-8      9.74kB ± 0%    9.72kB ± 0%   -0.16%  (p=0.000 n=10+10)
MergeJoiner/BothSidesRepeatInputSize=4096-8     14.3kB ± 0%    14.3kB ± 0%   -0.11%  (p=0.000 n=10+10)
MergeJoiner/BothSidesRepeatInputSize=65536-8    38.9kB ± 0%    38.9kB ± 0%   -0.04%  (p=0.000 n=10+10)

name                                          old allocs/op  new allocs/op  delta
MergeJoiner/InputSize=0-8                         15.0 ± 0%      15.0 ± 0%     ~     (all equal)
MergeJoiner/InputSize=4-8                         17.0 ± 0%      17.0 ± 0%     ~     (all equal)
MergeJoiner/InputSize=16-8                        17.0 ± 0%      17.0 ± 0%     ~     (all equal)
MergeJoiner/InputSize=256-8                       47.0 ± 0%      47.0 ± 0%     ~     (all equal)
MergeJoiner/InputSize=4096-8                       527 ± 0%       527 ± 0%     ~     (all equal)
MergeJoiner/InputSize=65536-8                    8.21k ± 0%     8.21k ± 0%     ~     (all equal)
MergeJoiner/OneSideRepeatInputSize=0-8            15.0 ± 0%      15.0 ± 0%     ~     (all equal)
MergeJoiner/OneSideRepeatInputSize=4-8            17.0 ± 0%      17.0 ± 0%     ~     (all equal)
MergeJoiner/OneSideRepeatInputSize=16-8           17.0 ± 0%      17.0 ± 0%     ~     (all equal)
MergeJoiner/OneSideRepeatInputSize=256-8          49.0 ± 0%      49.0 ± 0%     ~     (all equal)
MergeJoiner/OneSideRepeatInputSize=4096-8          536 ± 0%       536 ± 0%     ~     (all equal)
MergeJoiner/OneSideRepeatInputSize=65536-8       8.23k ± 0%     8.23k ± 0%     ~     (all equal)
MergeJoiner/BothSidesRepeatInputSize=0-8          15.0 ± 0%      15.0 ± 0%     ~     (all equal)
MergeJoiner/BothSidesRepeatInputSize=4-8          17.0 ± 0%      17.0 ± 0%     ~     (all equal)
MergeJoiner/BothSidesRepeatInputSize=16-8         17.0 ± 0%      17.0 ± 0%     ~     (all equal)
MergeJoiner/BothSidesRepeatInputSize=256-8        17.0 ± 0%      17.0 ± 0%     ~     (all equal)
MergeJoiner/BothSidesRepeatInputSize=4096-8       23.0 ± 0%      23.0 ± 0%     ~     (all equal)
MergeJoiner/BothSidesRepeatInputSize=65536-8      49.0 ± 0%      49.0 ± 0%     ~     (all equal)

name                                          old speed      new speed      delta
MergeJoiner/InputSize=4-8                     7.68MB/s ± 6%  8.02MB/s ± 9%   +4.43%  (p=0.037 n=9+10)
MergeJoiner/InputSize=16-8                    22.2MB/s ± 4%  22.6MB/s ± 5%     ~     (p=0.123 n=10+10)
MergeJoiner/InputSize=256-8                   46.4MB/s ± 3%  45.6MB/s ± 5%     ~     (p=0.231 n=9+9)
MergeJoiner/InputSize=4096-8                  49.2MB/s ± 4%  51.6MB/s ± 5%   +4.93%  (p=0.001 n=9+10)
MergeJoiner/InputSize=65536-8                 46.8MB/s ± 6%  49.3MB/s ± 9%     ~     (p=0.052 n=10+10)
MergeJoiner/OneSideRepeatInputSize=4-8        8.32MB/s ± 9%  8.31MB/s ± 4%     ~     (p=0.549 n=10+9)
MergeJoiner/OneSideRepeatInputSize=16-8       22.1MB/s ±24%  24.3MB/s ± 6%  +10.32%  (p=0.037 n=10+10)
MergeJoiner/OneSideRepeatInputSize=256-8      49.5MB/s ± 4%  51.0MB/s ± 5%   +3.03%  (p=0.003 n=10+10)
MergeJoiner/OneSideRepeatInputSize=4096-8     52.5MB/s ± 2%  42.8MB/s ±12%  -18.43%  (p=0.000 n=9+10)
MergeJoiner/OneSideRepeatInputSize=65536-8    43.3MB/s ± 3%  39.4MB/s ± 9%   -8.94%  (p=0.000 n=10+9)
MergeJoiner/BothSidesRepeatInputSize=4-8      8.98MB/s ± 4%  8.45MB/s ±15%   -5.96%  (p=0.005 n=9+10)
MergeJoiner/BothSidesRepeatInputSize=16-8     29.7MB/s ±13%  31.1MB/s ± 3%     ~     (p=0.549 n=10+9)
MergeJoiner/BothSidesRepeatInputSize=256-8     186MB/s ± 5%   177MB/s ± 8%   -4.73%  (p=0.004 n=9+10)
MergeJoiner/BothSidesRepeatInputSize=4096-8    299MB/s ± 3%   276MB/s ±21%     ~     (p=0.065 n=9+10)
MergeJoiner/BothSidesRepeatInputSize=65536-8   878MB/s ± 3%   898MB/s ± 1%   +2.22%  (p=0.001 n=10+9)

This change is

cockroach-teamcity · Oct 10, 2018

This change is

knz · Oct 10, 2018

knz approved these changes Oct 10, 2018

View changes

Can you also include the new benchmark diff in the commit message?

Reviewed 6 of 6 files at r1, 3 of 3 files at r2.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale)

knz · Oct 10, 2018

knz reviewed Oct 10, 2018

View changes

Reviewed 2 of 2 files at r3.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale)

pkg/util/mon/bytes_usage.go, line 480 at r3 (raw file):

}

// Empty shrinks the account to use 0 bytes, while maintaining the minAllocated

this comment refers to the now-inexistent minAllocated field
it does something fancy with reserved, this needs to be explained.

changangela · Oct 10, 2018

changangela reviewed Oct 10, 2018

View changes

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale)

pkg/util/mon/bytes_usage.go, line 480 at r3 (raw file):

Previously, knz (kena) wrote…

this comment refers to the now-inexistent minAllocated field

it does something fancy with reserved, this needs to be explained.

Oops, fixed.

You write this PR is meant to "significantly improve the merge joiner performance" yet the benchmark results you pasted above do not clearly show that. What do you think?

knz · Oct 11, 2018

You write this PR is meant to "significantly improve the merge joiner performance" yet the benchmark results you pasted above do not clearly show that. What do you think?

knz · Oct 11, 2018

knz reviewed Oct 11, 2018

View changes

Reviewed 1 of 1 files at r4.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale)

changangela · Oct 11, 2018

changangela reviewed Oct 11, 2018

View changes

I will benchmark this change against release-2.1 instead, because master technically already has this exact optimization.

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale)

LGTM thanks!

knz · Oct 11, 2018

LGTM thanks!

bors r+

changangela · Oct 11, 2018

bors r+

Build succeeded

GitHub CI (Cockroach)

craig · Oct 11, 2018

Build succeeded

GitHub CI (Cockroach)

Revert "Merge #30924"
This reverts commit 472d36f, reversing changes made to 6b912fa. Release note: None

99b8f67

changangela requested a review from knz Oct 10, 2018

changangela requested review from cockroachdb/distsql-prs as code owners Oct 10, 2018

changangela changed the title from sql: use sub-monitor for merge joiner to allocate reserved bytes to sql: add merge joiner benchmark and bytes usage optimization Oct 10, 2018

craig bot merged commit 17d7dc2 into cockroachdb:master Oct 11, 2018
3 checks passed

3 checks passed

GitHub CI (Cockroach) TeamCity build finished
Details

bors Build succeeded
Details

license/cla Contributor License Agreement is signed.
Details

cockroachdb/cockroach

sql: add merge joiner benchmark and bytes usage optimization #31216

changangela commented Oct 10, 2018 •

edited

changangela requested a review from knz Oct 10, 2018

changangela requested review from cockroachdb/distsql-prs as code owners Oct 10, 2018

This comment has been minimized.

cockroach-teamcity commented Oct 10, 2018

changangela changed the title from sql: use sub-monitor for merge joiner to allocate reserved bytes to sql: add merge joiner benchmark and bytes usage optimization Oct 10, 2018

knz approved these changes Oct 10, 2018

View changes

knz reviewed Oct 10, 2018

View changes

changangela reviewed Oct 10, 2018

View changes

This comment has been minimized.

knz commented Oct 11, 2018

knz reviewed Oct 11, 2018

View changes

changangela reviewed Oct 11, 2018

View changes

This comment has been minimized.

knz commented Oct 11, 2018

This comment has been minimized.

changangela commented Oct 11, 2018

craig bot pushed a commit that referenced this pull request Oct 11, 2018

This comment has been minimized.

craig bot commented Oct 11, 2018

craig bot merged commit `17d7dc2` into cockroachdb:master Oct 11, 2018
3 checks passed

3 checks passed

cockroachdb/cockroach

Join GitHub today

sql: add merge joiner benchmark and bytes usage optimization #31216

Conversation

changangela commented Oct 10, 2018 • edited

changangela requested a review from knz Oct 10, 2018

changangela requested review from cockroachdb/distsql-prs as code owners Oct 10, 2018

This comment has been minimized.

cockroach-teamcity Oct 10, 2018

cockroach-teamcity commented Oct 10, 2018

changangela changed the title from sql: use sub-monitor for merge joiner to allocate reserved bytes to sql: add merge joiner benchmark and bytes usage optimization Oct 10, 2018

knz approved these changes Oct 10, 2018 View changes

knz reviewed Oct 10, 2018 View changes

changangela reviewed Oct 10, 2018 View changes

This comment has been minimized.

knz Oct 11, 2018

knz commented Oct 11, 2018

knz reviewed Oct 11, 2018 View changes

changangela reviewed Oct 11, 2018 View changes

This comment has been minimized.

knz Oct 11, 2018

knz commented Oct 11, 2018

This comment has been minimized.

changangela Oct 11, 2018

changangela commented Oct 11, 2018

craig bot pushed a commit that referenced this pull request Oct 11, 2018

This comment has been minimized.

craig bot Oct 11, 2018

Build succeeded

craig bot commented Oct 11, 2018

Build succeeded

Hide details View details craig bot merged commit 17d7dc2 into cockroachdb:master Oct 11, 2018 3 checks passed

3 checks passed

changangela commented Oct 10, 2018 •

edited

knz approved these changes Oct 10, 2018

View changes

knz reviewed Oct 10, 2018

View changes

changangela reviewed Oct 10, 2018

View changes

knz reviewed Oct 11, 2018

View changes

changangela reviewed Oct 11, 2018

View changes

craig bot merged commit `17d7dc2` into cockroachdb:master Oct 11, 2018
3 checks passed