Skip to content

expression: implement vectorized evaluation for builtinLog1ArgSig#12285

Merged
Reminiscent merged 4 commits intopingcap:masterfrom
Handora:master
Sep 23, 2019
Merged

expression: implement vectorized evaluation for builtinLog1ArgSig#12285
Reminiscent merged 4 commits intopingcap:masterfrom
Handora:master

Conversation

@Handora
Copy link
Contributor

@Handora Handora commented Sep 19, 2019

What problem does this PR solve?

implement vectorized evaluation for builtinLog1ArgSig, for #12105

What is changed and how it works?

according to benchmark, about 2 times faster than before:

BenchmarkVectorizedBuiltinMathFunc/builtinLog1ArgSig-VecBuiltinFunc-8                     200000              8206 ns/op            0 B/op          0 allocs/op
BenchmarkVectorizedBuiltinMathFunc/builtinLog1ArgSig-NonVecBuiltinFunc-8                  100000             22445 ns/op            0 B/op          0 allocs/op

Check List

Tests

  • Unit test

@sre-bot sre-bot added the contribution This PR is from a community contributor. label Sep 19, 2019
@Handora Handora changed the title mplement vectorized evaluation for builtinLog1ArgSig Implement vectorized evaluation for builtinLog1ArgSig Sep 19, 2019
@CLAassistant
Copy link

CLAassistant commented Sep 19, 2019

CLA assistant check
All committers have signed the CLA.

@Handora
Copy link
Contributor Author

Handora commented Sep 19, 2019

/run-unit-test

@francis0407 francis0407 changed the title Implement vectorized evaluation for builtinLog1ArgSig expression: implement vectorized evaluation for builtinLog1ArgSig Sep 22, 2019
@qw4990 qw4990 requested review from Reminiscent and qw4990 and removed request for Reminiscent September 22, 2019 07:30
@qw4990 qw4990 requested a review from Reminiscent September 22, 2019 08:34
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

Copy link
Contributor

@SunRunAway SunRunAway left a comment

Choose a reason for hiding this comment

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

LGTM

@SunRunAway SunRunAway added the status/can-merge Indicates a PR has been approved by a committer. label Sep 22, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Sep 22, 2019

Your auto merge job has been accepted, waiting for 12286

@sre-bot
Copy link
Contributor

sre-bot commented Sep 22, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Sep 22, 2019

@Handora merge failed.

@codecov
Copy link

codecov bot commented Sep 22, 2019

Codecov Report

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

@@             Coverage Diff             @@
##             master     #12285   +/-   ##
===========================================
  Coverage   81.1093%   81.1093%           
===========================================
  Files           454        454           
  Lines         98927      98927           
===========================================
  Hits          80239      80239           
  Misses        12909      12909           
  Partials       5779       5779

@Handora
Copy link
Contributor Author

Handora commented Sep 22, 2019

/run-all-tests

@zz-jason
Copy link
Member

/rebuild

@zz-jason zz-jason added the status/LGT2 Indicates that a PR has LGTM 2. label Sep 23, 2019
@zz-jason
Copy link
Member

/run-unit-test

1 similar comment
@zz-jason
Copy link
Member

/run-unit-test

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.

7 participants