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.

Sign up for GitHub

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

Jump to bottom

expression: implement vectorized evaluation for builtinCastIntAsRealSig #12214

Merged
merged 12 commits into from
Sep 17, 2019
Merged

expression: implement vectorized evaluation for builtinCastIntAsRealSig #12214

merged 12 commits into from
Sep 17, 2019

Conversation

jacklightChen
Copy link
Contributor

@jacklightChen jacklightChen commented Sep 16, 2019 •
edited
Loading

What problem does this PR solve?

implement vectorized evaluation for builtinCastIntAsRealSig, for #12105

What is changed and how it works?

according to benchmark, about 5 times faster than before:

BenchmarkVectorizedBuiltinCastFunc/builtinCastIntAsRealSig-VecBuiltinFunc-8               500000              3357 ns/op            0 B/op          0 allocs/op
BenchmarkVectorizedBuiltinCastFunc/builtinCastIntAsRealSig-NonVecBuiltinFunc-8            100000             15234 ns/op            0 B/op          0 allocs/op

and if we use mergeNulls(),the performance become even better

BenchmarkVectorizedBuiltinCastFunc/builtinCastIntAsRealSig-VecBuiltinFunc-8               500000              2699 ns/op            0 B/op          0 allocs/op
BenchmarkVectorizedBuiltinCastFunc/builtinCastIntAsRealSig-NonVecBuiltinFunc-8            100000             16033 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 16, 2019
@codecov
Copy link

codecov bot commented Sep 16, 2019 •
edited
Loading

Codecov Report

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

@@            Coverage Diff            @@
##            master    #12214   +/-   ##
=========================================
  Coverage   81.169%   81.169%           
=========================================
  Files          454       454           
  Lines        98938     98938           
=========================================
  Hits         80307     80307           
  Misses       12856     12856           
  Partials      5775      5775

@jacklightChen jacklightChen changed the title implement vectorized evaluation for builtinCastIntAsRealSig expression: implement vectorized evaluation for builtinCastIntAsRealSig Sep 17, 2019
Reminiscent
Reminiscent reviewed Sep 17, 2019
View reviewed changes
Copy link
Contributor

@Reminiscent Reminiscent left a comment

Choose a reason for hiding this comment

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

LGTM

@Reminiscent Reminiscent added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 17, 2019
@francis0407
Copy link
Member

/rebuild

qw4990
qw4990 reviewed Sep 17, 2019
View reviewed changes
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.

Please resolve these conflicts.

qw4990
qw4990 approved these changes Sep 17, 2019
View reviewed changes
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 17, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Sep 17, 2019

Your auto merge job has been accepted, waiting for 12221, 12222

@sre-bot
Copy link
Contributor

sre-bot commented Sep 17, 2019

/run-all-tests

@sre-bot sre-bot merged commit 5225bd6 into pingcap:master Sep 17, 2019
@qw4990 qw4990 mentioned this pull request Sep 17, 2019
Closed
48 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Reminiscent Reminiscent Reminiscent left review comments

@qw4990 qw4990 qw4990 approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jacklightChen @francis0407 @sre-bot @qw4990 @Reminiscent