Skip to content
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

expression: implement vectorized evaluation for builtinCastStringAsRealSig #13445

Merged
merged 9 commits into from
Nov 18, 2019

Conversation

pingyu
Copy link
Contributor

@pingyu pingyu commented Nov 13, 2019

PCP #12105

What problem does this PR solve?

Implement vectorized evaluation for builtinCastStringAsRealSig at #12105

What is changed and how it works?

It gets faster about 20%.

Check List

Tests

  • Unit test
 > GO111MODULE=on go test -check.f TestVectorizedBuiltinCastFunc
PASS: builtin_cast_vec_test.go:132: testEvaluatorSuite.TestVectorizedBuiltinCastFunc	0.135s
OK: 1 passed
PASS
ok  	github.com/pingcap/tidb/expression	0.162s
  • Benchmark
> go test -v -benchmem -bench=BenchmarkVectorizedBuiltinCastFunc -run=BenchmarkVectorizedBuiltinCastFunc -args builtinCastStringAsRealSig
goos: darwin
goarch: amd64
pkg: github.com/pingcap/tidb/expression
BenchmarkVectorizedBuiltinCastFunc/builtinCastStringAsRealSig-VecBuiltinFunc-12         	   12048	     96351 ns/op	       0 B/op	       0 allocs/op
BenchmarkVectorizedBuiltinCastFunc/builtinCastStringAsRealSig-NonVecBuiltinFunc-12      	   10000	    116483 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/pingcap/tidb/expression	3.352s
  • Manual test (add detailed scripts or steps below)
mysql> select cast('1234.567' as real);
+--------------------------+
| cast('1234.567' as real) |
+--------------------------+
|                 1234.567 |
+--------------------------+
1 row in set (0.00 sec)

mysql> select cast(x'0102' as real);
+-----------------------+
| cast(x'0102' as real) |
+-----------------------+
|                   258 |
+-----------------------+
1 row in set (0.00 sec)

mysql> 

@pingyu pingyu requested a review from a team as a code owner November 13, 2019 18:09
@sre-bot
Copy link
Contributor

sre-bot commented Nov 13, 2019

Thanks for your contribution. If your PR get merged, you will be rewarded 50 points.

@sre-bot sre-bot added the contribution This PR is from a community contributor. label Nov 13, 2019
@ghost ghost requested review from wshwsh12 and XuHuaiyu and removed request for a team November 13, 2019 18:09
Copy link
Contributor

@XuHuaiyu XuHuaiyu 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 the status/LGT1 Indicates that a PR has LGTM 1. label Nov 16, 2019
@pingyu pingyu force-pushed the expression_vec_cast_string_as_real branch from e4c445c to 7a422cf Compare November 16, 2019 15:20
@codecov
Copy link

codecov bot commented Nov 16, 2019

Codecov Report

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

@@             Coverage Diff             @@
##             master     #13445   +/-   ##
===========================================
  Coverage   80.3183%   80.3183%           
===========================================
  Files           472        472           
  Lines        114894     114894           
===========================================
  Hits          92281      92281           
  Misses        15407      15407           
  Partials       7206       7206

@pingyu
Copy link
Contributor Author

pingyu commented Nov 16, 2019

/rebuild

@wshwsh12 wshwsh12 requested review from qw4990 and removed request for wshwsh12 November 17, 2019 17:31
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, WELL DONE!

@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 Nov 18, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Nov 18, 2019

Your auto merge job has been accepted, waiting for 13402

@sre-bot
Copy link
Contributor

sre-bot commented Nov 18, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Nov 18, 2019

@pingyu merge failed.

@qw4990 qw4990 merged commit 0b41fe9 into pingcap:master Nov 18, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Nov 18, 2019

@pingyu complete task #12105 and get 50 score, currerent score 150.

breezewish added a commit to breezewish/tidb that referenced this pull request Nov 18, 2019
qw4990 added a commit that referenced this pull request Nov 18, 2019
qw4990 pushed a commit that referenced this pull request Nov 18, 2019
XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019
XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019
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.

5 participants