Skip to content

expression: implement vectorized evaluation for builtinCastTimeAsRealSig#12421

Merged
ngaut merged 3 commits intopingcap:masterfrom
polyrabbit:vec-builtinCastTimeAsRealSig
Sep 27, 2019
Merged

expression: implement vectorized evaluation for builtinCastTimeAsRealSig#12421
ngaut merged 3 commits intopingcap:masterfrom
polyrabbit:vec-builtinCastTimeAsRealSig

Conversation

@polyrabbit
Copy link
Contributor

What problem does this PR solve?

Implement vectorized evaluation for builtinCastTimeAsRealSig, as listed in #12105.

What is changed and how it works?

According to benchmark, this change improves the performance, but not too much.

$ go test -v -benchmem -bench=BenchmarkVectorizedBuiltinCastFunc -run=BenchmarkVectorizedBuiltinCastFunc
...
BenchmarkVectorizedBuiltinCastFunc/builtinCastTimeAsRealSig-VecBuiltinFunc-8       	    2000	    651840 ns/op	  193505 B/op	   10298 allocs/op
BenchmarkVectorizedBuiltinCastFunc/builtinCastTimeAsRealSig-NonVecBuiltinFunc-8    	    2000	    700970 ns/op	  193505 B/op	   10298 allocs/op

Check List

Tests

  • Unit test

@sre-bot sre-bot added the contribution This PR is from a community contributor. label Sep 26, 2019
@CLAassistant
Copy link

CLAassistant commented Sep 26, 2019

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason zz-jason added component/expression status/LGT1 Indicates that a PR has LGTM 1. labels Sep 26, 2019
@zz-jason
Copy link
Member

@polyrabbit thanks for your contribution, please follow the Commit Message and Pull Request Style guide to reformat the PR title.

@polyrabbit polyrabbit changed the title Make builtinCastTimeAsRealSig support vectorized evaluation expression: make builtinCastTimeAsRealSig support vectorized evaluation Sep 26, 2019
@polyrabbit
Copy link
Contributor Author

Ah, I missed the rule. How does the new title look like?

@polyrabbit polyrabbit changed the title expression: make builtinCastTimeAsRealSig support vectorized evaluation expression: implement vectorized evaluation for builtinCastTimeAsRealSig Sep 27, 2019
Copy link
Contributor

@qw4990 qw4990 left a comment

Choose a reason for hiding this comment

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

LGTM

@qw4990 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 27, 2019
@polyrabbit polyrabbit requested a review from qw4990 September 27, 2019 01:45
@sre-bot
Copy link
Contributor

sre-bot commented Sep 27, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Sep 27, 2019

@polyrabbit merge failed.

@qw4990
Copy link
Contributor

qw4990 commented Sep 27, 2019

/run-unit-test

@ngaut ngaut merged commit 31cf75e into pingcap:master Sep 27, 2019
@codecov
Copy link

codecov bot commented Sep 27, 2019

Codecov Report

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

@@             Coverage Diff             @@
##             master     #12421   +/-   ##
===========================================
  Coverage   79.7522%   79.7522%           
===========================================
  Files           462        462           
  Lines        102125     102125           
===========================================
  Hits          81447      81447           
  Misses        14811      14811           
  Partials       5867       5867

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/expression contribution This PR is from a community contributor. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants