Skip to content

expression: implement vectorized evaluation for builtinLowerStr#12013

Merged
sre-bot merged 11 commits intopingcap:masterfrom
qw4990:vecexpr-lowerStr
Sep 11, 2019
Merged

expression: implement vectorized evaluation for builtinLowerStr#12013
sre-bot merged 11 commits intopingcap:masterfrom
qw4990:vecexpr-lowerStr

Conversation

@qw4990
Copy link
Contributor

@qw4990 qw4990 commented Sep 3, 2019

What problem does this PR solve?

Implement vectorized evaluation for builtinLowerStr.

What is changed and how it works?

Almost 2x faster than before:

BenchmarkVectorizedBuiltinFunc/builtinLowerSig-VecBuiltinFunc-12                           30000             54573 ns/op
BenchmarkVectorizedBuiltinFunc/builtinLowerSig-NonVecBuiltinFunc-12                        10000            109605 ns/op

Check List

Tests

  • Unit test

@codecov
Copy link

codecov bot commented Sep 3, 2019

Codecov Report

Merging #12013 into master will increase coverage by 0.0565%.
The diff coverage is 76.4705%.

@@              Coverage Diff               @@
##             master   #12013        +/-   ##
==============================================
+ Coverage   81.2734%   81.33%   +0.0565%     
==============================================
  Files           452      450         -2     
  Lines         96697    96583       -114     
==============================================
- Hits          78589    78551        -38     
+ Misses        12466    12398        -68     
+ Partials       5642     5634         -8

@qw4990
Copy link
Contributor Author

qw4990 commented Sep 3, 2019

/run-all-tests

@qw4990
Copy link
Contributor Author

qw4990 commented Sep 3, 2019

/run-unit-test

@SunRunAway SunRunAway removed their request for review September 4, 2019 08:57
@qw4990 qw4990 removed the request for review from Reminiscent September 5, 2019 08:10
@zz-jason
Copy link
Member

zz-jason commented Sep 9, 2019

to #12058

@qw4990 qw4990 requested review from XuHuaiyu and zz-jason September 10, 2019 12:12
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 the status/LGT1 Indicates that a PR has LGTM 1. label Sep 10, 2019
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

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

sre-bot commented Sep 11, 2019

/run-all-tests

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

Labels

component/expression 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