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 builtinSleepSig #14070

Closed

Conversation

shihongzhi
Copy link
Member

@shihongzhi shihongzhi commented Dec 16, 2019

What problem does this PR solve?

implement vectorized evaluation for builtinSleepSig , for #12103

What is changed and how it works?

$go test -v -benchmem -bench=BenchmarkVectorizedBuiltinMiscellaneousFunc -run=BenchmarkVectorizedBuiltinMiscellaneousFunc -args "builtinSleepSig"
goos: darwin
goarch: amd64
pkg: github.com/pingcap/tidb/expression
BenchmarkVectorizedBuiltinMiscellaneousFunc/builtinSleepSig-VecBuiltinFunc-4                   1        21000251592 ns/op            808 B/op         10 allocs/op
BenchmarkVectorizedBuiltinMiscellaneousFunc/builtinSleepSig-NonVecBuiltinFunc-4                1        21000123754 ns/op            208 B/op          3 allocs/op
PASS
ok      github.com/pingcap/tidb/expression      42.044s

Check List

Tests

  • Unit test

@qw4990 qw4990 added component/expression contribution This PR is from a community contributor. labels Dec 16, 2019
@shihongzhi
Copy link
Member Author

/rebuild

@shihongzhi shihongzhi marked this pull request as ready for review December 16, 2019 07:36
@shihongzhi shihongzhi requested a review from a team as a code owner December 16, 2019 07:36
@ghost ghost requested review from SunRunAway and XuHuaiyu and removed request for a team December 16, 2019 07:36
@codecov
Copy link

codecov bot commented Dec 16, 2019

Codecov Report

Merging #14070 into master will decrease coverage by 0.4837%.
The diff coverage is 75.5102%.

@@               Coverage Diff                @@
##             master     #14070        +/-   ##
================================================
- Coverage   80.6472%   80.1634%   -0.4838%     
================================================
  Files           488        483         -5     
  Lines        124659     121241      -3418     
================================================
- Hits         100534      97191      -3343     
+ Misses        16373      16307        -66     
+ Partials       7752       7743         -9

}
default:
if atomic.CompareAndSwapUint32(&sessVars.Killed, 1, 0) {
i64s[i] = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to return here when sessVars.Killed is set to 1.

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.

Add a test case like this for vecEval

@SunRunAway
Copy link
Contributor

Hi, please resolve this comment #14070 (review)

expression/builtin_miscellaneous.go Outdated Show resolved Hide resolved
expression/builtin_miscellaneous_vec_test.go Outdated Show resolved Hide resolved
expression/builtin_vectorized_test.go Outdated Show resolved Hide resolved
expression/builtin_vectorized_test.go Outdated Show resolved Hide resolved
expression/builtin_miscellaneous.go Outdated Show resolved Hide resolved
input := chunk.New([]*types.FieldType{tp}, 1, 1)
buf := chunk.NewColumn(types.NewFieldType(mysql.TypeLonglong), 1)
da := types.Datum{}
da.SetValue(a)
Copy link
Contributor

Choose a reason for hiding this comment

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

How about add another test for three rows with three arguments of 1 second ?

if rand.Float64() < g.nullRation {
return nil
}
return math.Floor(rand.Float64() * 10)
Copy link
Contributor

Choose a reason for hiding this comment

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

It's still too long, how about keep it in a set of [0, 0.1]?

@@ -31,7 +31,7 @@ func (g *sleepTimeGener) gen() interface{} {
if rand.Float64() < g.nullRation {
return nil
}
return math.Floor(rand.Float64() * 10)
return math.Floor(rand.Float64())
Copy link
Contributor

@SunRunAway SunRunAway Dec 24, 2019

Choose a reason for hiding this comment

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

if rand.Float64() < 0.5 {
    return 0
}
return 0.1

@SunRunAway
Copy link
Contributor

Hi, @shihongzhi Please fix the unit test.

@sre-bot
Copy link
Contributor

sre-bot commented Jan 2, 2020

@shihongzhi, please update your pull request.

1 similar comment
@sre-bot
Copy link
Contributor

sre-bot commented Jan 9, 2020

@shihongzhi, please update your pull request.

@sre-bot
Copy link
Contributor

sre-bot commented Jan 25, 2020

@shihongzhi PR closed due to no update for a long time. Feel free to reopen it anytime.

@sre-bot sre-bot closed this Jan 25, 2020
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants