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 builtinMakeDateSig #13305

Merged
merged 20 commits into from
Nov 11, 2019

Conversation

ekalinin
Copy link
Contributor

@ekalinin ekalinin commented Nov 8, 2019

PCP #12101

What problem does this PR solve?

This PR implements vectorized builtinMakeDateSig

What is changed and how it works?

➜ make vectorized-bench VB_FILE=Time VB_FUNC=builtinMakeDateSig
cd ./expression && \
	go test -v -benchmem \
		-bench=BenchmarkVectorizedBuiltinTimeFunc \
		-run=BenchmarkVectorizedBuiltinTimeFunc \
		-args "builtinMakeDateSig"
goos: linux
goarch: amd64
pkg: github.com/pingcap/tidb/expression
BenchmarkVectorizedBuiltinTimeFuncGenerated-12    	1000000000	         0.00908 ns/op	       0 B/op	       0 allocs/op
BenchmarkVectorizedBuiltinTimeFunc/builtinMakeDateSig-VecBuiltinFunc-12         	   12246	     94084 ns/op	       0 B/op	       0 allocs/op
BenchmarkVectorizedBuiltinTimeFunc/builtinMakeDateSig-NonVecBuiltinFunc-12      	    9542	    125687 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/pingcap/tidb/expression	3.337s

Check List

Tests

  • Unit test
  • Integration test

@ekalinin ekalinin requested a review from a team as a code owner November 8, 2019 21:45
@sre-bot
Copy link
Contributor

sre-bot commented Nov 8, 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 8, 2019
@ghost ghost requested review from SunRunAway and wshwsh12 and removed request for a team November 8, 2019 21:45
@codecov
Copy link

codecov bot commented Nov 9, 2019

Codecov Report

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

@@             Coverage Diff             @@
##             master     #13305   +/-   ##
===========================================
  Coverage   80.1427%   80.1427%           
===========================================
  Files           469        469           
  Lines        112543     112543           
===========================================
  Hits          90195      90195           
  Misses        15342      15342           
  Partials       7006       7006

expression/builtin_time_vec.go Outdated Show resolved Hide resolved
expression/builtin_time_vec_test.go Outdated Show resolved Hide resolved
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
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. labels Nov 11, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Nov 11, 2019

Your auto merge job has been accepted, waiting for 13319

@qw4990
Copy link
Contributor

qw4990 commented Nov 11, 2019

/run-all-tests

1 similar comment
@sre-bot
Copy link
Contributor

sre-bot commented Nov 11, 2019

/run-all-tests

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

sre-bot commented Nov 11, 2019

@ekalinin complete task #12101 and get 50 score, currerent score 400.

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/LGT1 Indicates that a PR has LGTM 1. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants