expression: go generate vectorized addtime functions #12224

SunRunAway · 2019-09-17T09:34:02Z

What problem does this PR solve?

Part of #12058

Use go template to generate vectorized ADDTIME functions.

builtinAddDatetimeAndDurationSig
builtinAddDatetimeAndStringSig
builtinAddDurationAndDurationSig
builtinAddDurationAndStringSig
builtinAddStringAndDurationSig
builtinAddStringAndStringSig
builtinAddDateAndDurationSig
builtinAddDateAndStringSig
builtinAddTimeDateTimeNullSig
builtinAddTimeStringNullSig
builtinAddTimeDurationNullSig

By hand write a cast function for unit test.

builtinCastTimeAsDurationSig

What is changed and how it works?

addtime

name                                                                         old time/op    new time/op    delta
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddDatetimeAndDurationSig12     174µs ±16%      62µs ± 4%   -64.28%  (p=0.008 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddDatetimeAndStringSig12      1.37ms ± 4%    1.19ms ± 7%   -13.15%  (p=0.008 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddDurationAndDurationSig12    74.5µs ±79%     5.3µs ± 9%   -92.89%  (p=0.008 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddDurationAndStringSig12      1.17ms ± 3%    1.10ms ± 3%    -5.56%  (p=0.016 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddStringAndDurationSig12      1.48ms ± 2%    1.41ms ± 3%    -4.83%  (p=0.008 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddStringAndStringSig12        2.33ms ± 3%    2.25ms ± 3%      ~     (p=0.056 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddDateAndDurationSig12         409µs ±14%     341µs ± 0%   -16.66%  (p=0.016 n=5+4)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddDateAndStringSig12          1.73ms ± 4%    1.68ms ± 6%      ~     (p=0.310 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddTimeDateTimeNullSig12        142µs ±68%       0µs ± 3%   -99.95%  (p=0.008 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddTimeStringNullSig12         43.0µs ±42%     2.1µs ± 3%   -95.10%  (p=0.008 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddTimeDurationNullSig12       22.5µs ±11%     0.1µs ± 3%   -99.69%  (p=0.016 n=4+5)
VectorizedBuiltinTimeFuncGenerated/builtinAddDatetimeAndDurationSig12           100µs ± 5%      62µs ± 4%   -37.26%  (p=0.008 n=5+5)
VectorizedBuiltinTimeFuncGenerated/builtinAddDatetimeAndStringSig12            1.47ms ± 3%    1.41ms ± 1%      ~     (p=0.056 n=5+5)
VectorizedBuiltinTimeFuncGenerated/builtinAddDurationAndDurationSig12          29.7µs ± 2%     4.8µs ± 5%   -83.69%  (p=0.008 n=5+5)
VectorizedBuiltinTimeFuncGenerated/builtinAddDurationAndStringSig12            1.18ms ± 7%    1.12ms ± 8%      ~     (p=0.095 n=5+5)
VectorizedBuiltinTimeFuncGenerated/builtinAddStringAndDurationSig12            1.53ms ± 5%    1.57ms ± 8%      ~     (p=0.841 n=5+5)
VectorizedBuiltinTimeFuncGenerated/builtinAddStringAndStringSig12              2.46ms ± 3%    2.43ms ± 6%      ~     (p=0.690 n=5+5)
VectorizedBuiltinTimeFuncGenerated/builtinAddDateAndDurationSig12               352µs ± 6%     315µs ±10%   -10.59%  (p=0.032 n=5+5)
VectorizedBuiltinTimeFuncGenerated/builtinAddDateAndStringSig12                1.58ms ± 4%    1.66ms ±11%      ~     (p=0.421 n=5+5)
VectorizedBuiltinTimeFuncGenerated/builtinAddTimeDateTimeNullSig12             12.3µs ± 3%     0.1µs ± 3%   -99.48%  (p=0.008 n=5+5)
VectorizedBuiltinTimeFuncGenerated/builtinAddTimeStringNullSig12               5.14µs ± 3%    2.02µs ± 5%   -60.80%  (p=0.008 n=5+5)
VectorizedBuiltinTimeFuncGenerated/builtinAddTimeDurationNullSig12             7.57µs ± 2%    0.06µs ± 3%   -99.17%  (p=0.008 n=5+5)

name                                                                         old alloc/op   new alloc/op   delta
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddDatetimeAndDurationSig12     106kB ±24%       0kB       -100.00%  (p=0.008 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddDatetimeAndStringSig12       210kB ±11%      96kB ± 0%   -54.31%  (p=0.008 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddDurationAndDurationSig12   65.5kB ±136%     0.0kB       -100.00%  (p=0.008 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddDurationAndStringSig12       147kB ± 8%      97kB ± 0%   -34.12%  (p=0.008 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddStringAndDurationSig12       271kB ±10%     190kB ± 0%   -29.88%  (p=0.008 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddStringAndStringSig12         386kB ± 5%     292kB ± 0%   -24.28%  (p=0.008 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddDateAndDurationSig12         189kB ± 8%     101kB ± 0%   -46.54%  (p=0.008 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddDateAndStringSig12           318kB ± 5%     212kB ± 0%   -33.45%  (p=0.008 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddTimeDateTimeNullSig12        126kB ±36%       0kB       -100.00%  (p=0.008 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddTimeStringNullSig12         44.4kB ±22%     0.0kB       -100.00%  (p=0.008 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddTimeDurationNullSig12      39.3kB ±115%     0.0kB       -100.00%  (p=0.008 n=5+5)
VectorizedBuiltinTimeFuncGenerated/builtinAddDatetimeAndDurationSig12           0.00B          0.00B           ~     (all equal)
VectorizedBuiltinTimeFuncGenerated/builtinAddDatetimeAndStringSig12             102kB ± 0%     102kB ± 0%      ~     (p=0.841 n=5+5)
VectorizedBuiltinTimeFuncGenerated/builtinAddDurationAndDurationSig12           0.00B          0.00B           ~     (all equal)
VectorizedBuiltinTimeFuncGenerated/builtinAddDurationAndStringSig12            96.2kB ± 0%    96.4kB ± 0%      ~     (p=0.690 n=5+5)
VectorizedBuiltinTimeFuncGenerated/builtinAddStringAndDurationSig12             198kB ± 0%     198kB ± 0%      ~     (p=0.310 n=5+5)
VectorizedBuiltinTimeFuncGenerated/builtinAddStringAndStringSig12               290kB ± 0%     290kB ± 0%      ~     (p=0.841 n=5+5)
VectorizedBuiltinTimeFuncGenerated/builtinAddDateAndDurationSig12              98.2kB ± 0%    98.2kB ± 0%    -0.00%  (p=0.000 n=5+4)
VectorizedBuiltinTimeFuncGenerated/builtinAddDateAndStringSig12                 211kB ± 0%     210kB ± 0%      ~     (p=0.222 n=5+5)
VectorizedBuiltinTimeFuncGenerated/builtinAddTimeDateTimeNullSig12              0.00B          0.00B           ~     (all equal)
VectorizedBuiltinTimeFuncGenerated/builtinAddTimeStringNullSig12                0.00B          0.00B           ~     (all equal)
VectorizedBuiltinTimeFuncGenerated/builtinAddTimeDurationNullSig12              0.00B          0.00B           ~     (all equal)

name                                                                         old allocs/op  new allocs/op  delta
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddDatetimeAndDurationSig12      0.00           0.00           ~     (all equal)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddDatetimeAndStringSig12       5.82k ± 0%     5.82k ± 0%      ~     (all equal)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddDurationAndDurationSig12      0.00           0.00           ~     (all equal)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddDurationAndStringSig12       5.88k ± 0%     5.88k ± 0%      ~     (all equal)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddStringAndDurationSig12       9.26k ± 0%     9.26k ± 0%    +0.01%  (p=0.008 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddStringAndStringSig12         15.3k ± 0%     15.3k ± 0%    +0.01%  (p=0.008 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddDateAndDurationSig12         3.96k ± 0%     3.96k ± 0%      ~     (all equal)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddDateAndStringSig12           10.5k ± 0%     10.5k ± 0%    +0.01%  (p=0.008 n=5+5)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddTimeDateTimeNullSig12         0.00           0.00           ~     (all equal)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddTimeStringNullSig12           0.00           0.00           ~     (all equal)
VectorizedBuiltinTimeEvalOneVecGenerated/builtinAddTimeDurationNullSig12         0.00           0.00           ~     (all equal)
VectorizedBuiltinTimeFuncGenerated/builtinAddDatetimeAndDurationSig12            0.00           0.00           ~     (all equal)
VectorizedBuiltinTimeFuncGenerated/builtinAddDatetimeAndStringSig12             6.18k ± 0%     6.18k ± 0%      ~     (all equal)
VectorizedBuiltinTimeFuncGenerated/builtinAddDurationAndDurationSig12            0.00           0.00           ~     (all equal)
VectorizedBuiltinTimeFuncGenerated/builtinAddDurationAndStringSig12             5.84k ± 0%     5.84k ± 0%      ~     (all equal)
VectorizedBuiltinTimeFuncGenerated/builtinAddStringAndDurationSig12             9.65k ± 0%     9.65k ± 0%      ~     (all equal)
VectorizedBuiltinTimeFuncGenerated/builtinAddStringAndStringSig12               15.1k ± 0%     15.1k ± 0%      ~     (all equal)
VectorizedBuiltinTimeFuncGenerated/builtinAddDateAndDurationSig12               3.86k ± 0%     3.86k ± 0%      ~     (all equal)
VectorizedBuiltinTimeFuncGenerated/builtinAddDateAndStringSig12                 10.4k ± 0%     10.4k ± 0%      ~     (all equal)
VectorizedBuiltinTimeFuncGenerated/builtinAddTimeDateTimeNullSig12               0.00           0.00           ~     (all equal)
VectorizedBuiltinTimeFuncGenerated/builtinAddTimeStringNullSig12                 0.00           0.00           ~     (all equal)
VectorizedBuiltinTimeFuncGenerated/builtinAddTimeDurationNullSig12               0.00           0.00           ~     (all equal)

cast:

name                                                        old time/op    new time/op    delta
VectorizedBuiltinCastFunc/builtinCastTimeAsDurationSig--12    48.7µs ± 4%    32.0µs ± 4%  -34.27%  (p=0.008 n=5+5)

name                                                        old alloc/op   new alloc/op   delta
VectorizedBuiltinCastFunc/builtinCastTimeAsDurationSig--12     0.00B          0.00B          ~     (all equal)

name                                                        old allocs/op  new allocs/op  delta
VectorizedBuiltinCastFunc/builtinCastTimeAsDurationSig--12      0.00           0.00          ~     (all equal)

Check List

Tests

Unit test

codecov · 2019-09-17T10:03:27Z

Codecov Report

Merging #12224 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #12224   +/-   ##
===========================================
  Coverage   79.7697%   79.7697%           
===========================================
  Files           462        462           
  Lines        102337     102337           
===========================================
  Hits          81634      81634           
  Misses        14832      14832           
  Partials       5871       5871

SunRunAway · 2019-09-17T10:28:32Z

/run-all-tests

qw4990 · 2019-09-17T11:38:00Z

/run-unit-test

zz-jason · 2019-09-17T13:29:03Z

in the performance benchmarks for add_time:

name                                                                     old time/op    new time/op     delta
VectorizedBuiltinTimeFuncGenerated/builtinAddTimeStringNullSig--12         2.98µs ± 4%    77.28µs ±91%  +2494.20%  (p=0.008 n=5+5)

seems there is a performance regression?

SunRunAway · 2019-09-18T04:18:18Z

in the performance benchmarks for add_time:

name                                                                     old time/op    new time/op     delta
VectorizedBuiltinTimeFuncGenerated/builtinAddTimeStringNullSig--12         2.98µs ± 4%    77.28µs ±91%  +2494.20%  (p=0.008 n=5+5)

seems there is a performance regression?

I think it's the problem of the benchmark framework.
vecEvalString needs to apply its results into a *chunk.Column in benchmarkVecBuiltinFunc, meanwhile evalString does not need in NonVecBuiltinFunc

@qw4990 will fix it later.

qw4990 · 2019-09-18T05:02:02Z

in the performance benchmarks for add_time:
name                                                                     old time/op    new time/op     delta
VectorizedBuiltinTimeFuncGenerated/builtinAddTimeStringNullSig--12         2.98µs ± 4%    77.28µs ±91%  +2494.20%  (p=0.008 n=5+5)
seems there is a performance regression?
I think it's the problem of the benchmark framework.
vecEvalString needs to apply its results into a *chunk.Column in benchmarkVecBuiltinFunc, meanwhile evalString does not need in NonVecBuiltinFunc

I will fix it soon.

expression/builtin_cast_vec.go

qw4990 · 2019-09-18T12:36:17Z

in the performance benchmarks for add_time:
name                                                                     old time/op    new time/op     delta
VectorizedBuiltinTimeFuncGenerated/builtinAddTimeStringNullSig--12         2.98µs ± 4%    77.28µs ±91%  +2494.20%  (p=0.008 n=5+5)
seems there is a performance regression?
I think it's the problem of the benchmark framework.
vecEvalString needs to apply its results into a *chunk.Column in benchmarkVecBuiltinFunc, meanwhile evalString does not need in NonVecBuiltinFunc
I will fix it soon.

It is fixed, so please update the benchmark results.

expression/builtin_cast_vec.go

SunRunAway · 2019-09-21T10:58:17Z

Please review expression: support customizing FieldTypes of arguments for benchmark framework #12291 firstly.

SunRunAway · 2019-09-24T10:25:54Z

/run-unit-test

qw4990

Almost LGTM, but please resolve conflicts. @SunRunAway

SunRunAway · 2019-09-26T03:41:00Z

Almost LGTM, but please resolve conflicts. @SunRunAway

Rebased, conflicts resolved in builtin_cast_vec.go, builtin_cast_vec_test.go and builtin_time_vec_test.go

zyxbest · 2019-09-26T03:59:07Z

/run-unit-test

qw4990

LGTM

sre-bot · 2019-09-26T07:22:34Z

/run-all-tests

sre-bot · 2019-09-26T07:33:32Z

@SunRunAway merge failed.

SunRunAway · 2019-09-26T08:25:13Z

/merge

sre-bot · 2019-09-26T08:25:30Z

/run-all-tests

SunRunAway changed the title ~~go generate vectorized addtime functions~~ executor: go generate vectorized addtime functions Sep 17, 2019

SunRunAway force-pushed the addTimeFunctionClass2 branch 2 times, most recently from 667a001 to 165d5ec Compare September 17, 2019 09:44

SunRunAway added the component/expression label Sep 17, 2019

SunRunAway requested review from Reminiscent, qw4990, XuHuaiyu and zz-jason and removed request for Reminiscent September 17, 2019 09:51

Reminiscent reviewed Sep 18, 2019

View reviewed changes

expression/builtin_cast_vec.go Show resolved Hide resolved

SunRunAway force-pushed the addTimeFunctionClass2 branch from 3e561d1 to 226befe Compare September 18, 2019 07:40

SunRunAway requested a review from Reminiscent September 18, 2019 07:40

SunRunAway changed the title ~~executor: go generate vectorized addtime functions~~ expression: go generate vectorized addtime functions Sep 18, 2019

qw4990 reviewed Sep 18, 2019

View reviewed changes

expression/builtin_cast_vec.go Show resolved Hide resolved

SunRunAway added the status/WIP label Sep 21, 2019

SunRunAway removed request for XuHuaiyu, zz-jason and Reminiscent September 21, 2019 10:54

SunRunAway mentioned this pull request Sep 21, 2019

expression: support customizing FieldTypes of arguments for benchmark framework #12291

Merged

SunRunAway force-pushed the addTimeFunctionClass2 branch 2 times, most recently from 2aa3c49 to 71233d5 Compare September 23, 2019 04:22

SunRunAway force-pushed the addTimeFunctionClass2 branch from 0e64be2 to c4b7fbf Compare September 24, 2019 09:38

qw4990 reviewed Sep 25, 2019

View reviewed changes

SunRunAway added 8 commits September 26, 2019 11:33

expression: go generate vectorized addtime functions

169f370

adjust

8c3f1d9

we should call result.ReserveXXX

44cea37

update comment

eae1090

address comment

cbb07e9

address comment

5c56bfd

add codecov exception

5923423

fix

adfe565

SunRunAway force-pushed the addTimeFunctionClass2 branch from c4b7fbf to adfe565 Compare September 26, 2019 03:35

resolve conflict

89ffcd7

add comment

b8813c3

SunRunAway requested a review from qw4990 September 26, 2019 03:49

qw4990 approved these changes Sep 26, 2019

View reviewed changes

qw4990 added status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 26, 2019

Merge branch 'master' into addTimeFunctionClass2

c871842

Merge branch 'master' into addTimeFunctionClass2

e3e577f

sre-bot merged commit 35e308f into pingcap:master Sep 26, 2019

SunRunAway deleted the addTimeFunctionClass2 branch September 26, 2019 08:30

qw4990 mentioned this pull request Nov 20, 2019

expression: implement vectorized evaluation for builtinAddDateStringStringSig #13476

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

expression: go generate vectorized addtime functions #12224

expression: go generate vectorized addtime functions #12224

SunRunAway commented Sep 17, 2019 •

edited

Loading

codecov bot commented Sep 17, 2019 •

edited

Loading

SunRunAway commented Sep 17, 2019

qw4990 commented Sep 17, 2019

zz-jason commented Sep 17, 2019

SunRunAway commented Sep 18, 2019 •

edited

Loading

qw4990 commented Sep 18, 2019

qw4990 commented Sep 18, 2019

SunRunAway commented Sep 21, 2019 •

edited

Loading

SunRunAway commented Sep 24, 2019

qw4990 left a comment

SunRunAway commented Sep 26, 2019

zyxbest commented Sep 26, 2019

qw4990 left a comment

sre-bot commented Sep 26, 2019

sre-bot commented Sep 26, 2019

SunRunAway commented Sep 26, 2019

sre-bot commented Sep 26, 2019

expression: go generate vectorized addtime functions #12224

expression: go generate vectorized addtime functions #12224

Conversation

SunRunAway commented Sep 17, 2019 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

codecov bot commented Sep 17, 2019 • edited Loading

Codecov Report

SunRunAway commented Sep 17, 2019

qw4990 commented Sep 17, 2019

zz-jason commented Sep 17, 2019

SunRunAway commented Sep 18, 2019 • edited Loading

qw4990 commented Sep 18, 2019

qw4990 commented Sep 18, 2019

SunRunAway commented Sep 21, 2019 • edited Loading

SunRunAway commented Sep 24, 2019

qw4990 left a comment

Choose a reason for hiding this comment

SunRunAway commented Sep 26, 2019

zyxbest commented Sep 26, 2019

qw4990 left a comment

Choose a reason for hiding this comment

sre-bot commented Sep 26, 2019

sre-bot commented Sep 26, 2019

SunRunAway commented Sep 26, 2019

sre-bot commented Sep 26, 2019

SunRunAway commented Sep 17, 2019 •

edited

Loading

codecov bot commented Sep 17, 2019 •

edited

Loading

SunRunAway commented Sep 18, 2019 •

edited

Loading

SunRunAway commented Sep 21, 2019 •

edited

Loading