-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
colexecjoin: optimize building output on the left in merge joiner #78101
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor suggestion for better clarity in the template, otherwise . Nice optimization!
Reviewed 4 of 16 files at r1.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @michae2 and @yuzefovich)
pkg/sql/colexec/colexecjoin/mergejoiner_tmpl.go, line 752 at r1 (raw file):
for i := 0; i < toAppend; i++ { // {{if .IsBytesLike}} outCol.Copy(srcCol, outStartIdx+i, srcStartIdx)
Just to clarify, anything that is bytes like is not sliceable? Otherwise it seems like using outStartIdx+i
would potentially go out of bounds after outCol := outCol[outStartIdx:]
above.
Consider reordering the statements before the loop to make this clearer in the template and mirror the ordering in the loop, e.g.:
// {{if not .IsBytesLike}}
// {{if .Sliceable}}
...
// {{end}}
...
// {{end}}
This commit updates the way we're building output in the merge joiner from the left input when building directly from the left batch (i.e. not from the buffered group). There, we need to repeat a single tuple `toAppend` times, so we do it in a loop. This commit adds the optimization of using `Bytes.Copy` for the bytes-like types as well as BCE for sliceable types. ``` MergeJoiner/rows=32-24 29.3MB/s ± 3% 29.5MB/s ± 3% ~ (p=0.684 n=10+10) MergeJoiner/rows=512-24 79.4MB/s ± 2% 77.8MB/s ± 3% -1.91% (p=0.043 n=10+10) MergeJoiner/rows=4096-24 192MB/s ± 2% 189MB/s ± 1% -1.36% (p=0.029 n=10+10) MergeJoiner/rows=32768-24 278MB/s ± 1% 275MB/s ± 0% -1.30% (p=0.000 n=10+10) MergeJoiner/oneSideRepeat-rows=32-24 37.3MB/s ± 3% 38.0MB/s ± 2% +1.78% (p=0.029 n=10+10) MergeJoiner/oneSideRepeat-rows=512-24 212MB/s ± 1% 215MB/s ± 2% +1.42% (p=0.003 n=9+10) MergeJoiner/oneSideRepeat-rows=4096-24 765MB/s ± 4% 770MB/s ± 3% ~ (p=0.436 n=10+10) MergeJoiner/oneSideRepeat-rows=32768-24 1.22GB/s ± 2% 1.23GB/s ± 2% ~ (p=0.393 n=10+10) MergeJoiner/bothSidesRepeat-rows=32-24 22.7MB/s ± 2% 22.9MB/s ± 2% ~ (p=0.203 n=9+10) MergeJoiner/bothSidesRepeat-rows=512-24 102MB/s ± 4% 104MB/s ± 2% +2.38% (p=0.011 n=10+10) MergeJoiner/bothSidesRepeat-rows=4096-24 117MB/s ± 1% 127MB/s ± 1% +9.11% (p=0.000 n=10+9) MergeJoiner/bothSidesRepeat-rows=32768-24 59.2MB/s ± 1% 67.1MB/s ± 1% +13.48% (p=0.000 n=10+10) ``` Release note: None
7f688fd
to
d82affe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TFTR!
bors r+
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @michae2 and @rharding6373)
pkg/sql/colexec/colexecjoin/mergejoiner_tmpl.go, line 752 at r1 (raw file):
anything that is bytes like is not sliceable?
Yes. All vectors currently are sliceable except for bytes-like and datum-backed. I like your suggestion, done.
Build succeeded: |
This commit updates the way we're building output in the merge joiner
from the left input when building directly from the left batch (i.e. not
from the buffered group). There, we need to repeat a single tuple
toAppend
times, so we do it in a loop. This commit adds theoptimization of using
Bytes.Copy
for the bytes-like types as well asBCE for sliceable types.
Release note: None