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 builtinYearWeekWithModeSig #13328

Merged
merged 5 commits into from
Nov 11, 2019
Merged

expression: implement vectorized evaluation for builtinYearWeekWithModeSig #13328

merged 5 commits into from
Nov 11, 2019

Conversation

chenlx0
Copy link
Contributor

@chenlx0 chenlx0 commented Nov 10, 2019

PCP #12103

What problem does this PR solve?

implement vectorized evaluation for builtinYearWeekWithModeSig

What is changed and how it works?

$ make vectorized-bench VB_FILE=Time VB_FUNC=builtinYearWeekWithModeSig
cd ./expression && \
                go test -v -benchmem \
                        -bench=BenchmarkVectorizedBuiltinTimeFunc \
                        -run=BenchmarkVectorizedBuiltinTimeFunc \
                        -args "builtinYearWeekWithModeSig"
goos: darwin
goarch: amd64
pkg: github.com/pingcap/tidb/expression
BenchmarkVectorizedBuiltinTimeFuncGenerated-8           1000000000               0.00732 ns/op         0 B/op          0 allocs/op
BenchmarkVectorizedBuiltinTimeFunc/builtinYearWeekWithModeSig-VecBuiltinFunc-8             28470             41544 ns/op           29192 B/op          8 allocs/op
BenchmarkVectorizedBuiltinTimeFunc/builtinYearWeekWithModeSig-NonVecBuiltinFunc-8          19538             60255 ns/op               0 B/op          0 allocs/op
PASS
ok      github.com/pingcap/tidb/expression      3.505s

Check List

Tests

  • Unit test

@chenlx0 chenlx0 requested a review from a team as a code owner November 10, 2019 10:53
@sre-bot
Copy link
Contributor

sre-bot commented Nov 10, 2019

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

@ghost ghost requested review from qw4990 and SunRunAway and removed request for a team November 10, 2019 10:53
@sre-bot sre-bot added the contribution This PR is from a community contributor. label Nov 10, 2019
@codecov
Copy link

codecov bot commented Nov 10, 2019 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@60bfdf5). Click here to learn what that means.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #13328   +/-   ##
===========================================
  Coverage          ?   80.3043%           
===========================================
  Files             ?        469           
  Lines             ?     112964           
  Branches          ?          0           
===========================================
  Hits              ?      90715           
  Misses            ?      15247           
  Partials          ?       7002

@chenlx0
Copy link
Contributor Author

chenlx0 commented Nov 11, 2019

/run-all-tests

b41sh
b41sh reviewed Nov 11, 2019
View reviewed changes
expression/builtin_time_vec.go Outdated
}

// evalInt evals YEARWEEK(date,mode).
Copy link
Member

Choose a reason for hiding this comment

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

vecEvalInt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

SunRunAway
SunRunAway reviewed Nov 11, 2019
View reviewed changes
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/LGT1 Indicates that a PR has LGTM 1. label Nov 11, 2019
@b41sh
Copy link
Member

b41sh commented Nov 11, 2019

LGTM

@SunRunAway
Copy link
Contributor

/merge

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

sre-bot commented Nov 11, 2019

Your auto merge job has been accepted, waiting for 13360, 13363

@SunRunAway SunRunAway added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Nov 11, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Nov 11, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Nov 11, 2019

@chenlx0 merge failed.

@qw4990
Copy link
Contributor

qw4990 commented Nov 11, 2019

/run-all-tests

@qw4990 qw4990 merged commit 88e96eb into pingcap:master Nov 11, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Nov 11, 2019

Team blueshit complete task #12103 and get 50 score, currerent score 100.

XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019
@chenlx0 @XiaTianliang
expression: implement vectorized evaluation for `builtinYearWeekWithM…
71a6ba8 
…odeSig` (pingcap#13328)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@b41sh b41sh b41sh left review comments

@SunRunAway SunRunAway SunRunAway approved these changes

@qw4990 qw4990 Awaiting requested review from qw4990

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.
6 participants
@chenlx0 @sre-bot @b41sh @SunRunAway @qw4990 @ngaut