Skip to content

expression: implement vectorized evaluation for builtinJSONObjectSig#12663

Merged
qw4990 merged 17 commits intopingcap:masterfrom
js00070:pr0
Oct 23, 2019
Merged

expression: implement vectorized evaluation for builtinJSONObjectSig#12663
qw4990 merged 17 commits intopingcap:masterfrom
js00070:pr0

Conversation

@js00070
Copy link
Contributor

@js00070 js00070 commented Oct 12, 2019

What problem does this PR solve?

implement vectorized builtinJSONObjectSig mentioned at #12104

What is changed and how it works?

only about 5% faster. maybe there's room for improvement.

goos: linux
goarch: amd64
pkg: github.com/pingcap/tidb/expression
BenchmarkVectorizedBuiltinJSONFunc/builtinJSONObjectSig-VecBuiltinFunc-4                     200           8514648 ns/op         3668925 B/op      29167 allocs/op
BenchmarkVectorizedBuiltinJSONFunc/builtinJSONObjectSig-NonVecBuiltinFunc-4                  200           8954836 ns/op         3660707 B/op      29166 allocs/op
PASS
ok      github.com/pingcap/tidb/expression      5.327s

Check List

Tests

  • Unit test

@sre-bot sre-bot added the contribution This PR is from a community contributor. label Oct 12, 2019
@CLAassistant
Copy link

CLAassistant commented Oct 12, 2019

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Oct 12, 2019

Codecov Report

Merging #12663 into master will decrease coverage by 0.5221%.
The diff coverage is 70.2127%.

@@               Coverage Diff                @@
##             master     #12663        +/-   ##
================================================
- Coverage   80.4874%   79.9652%   -0.5222%     
================================================
  Files           465        465                
  Lines        110216     107074      -3142     
================================================
- Hits          88710      85622      -3088     
- Misses        14979      14996        +17     
+ Partials       6527       6456        -71

@foreyes
Copy link
Contributor

foreyes commented Oct 12, 2019

/run-all-tests

@zz-jason
Copy link
Member

/run-common-test
/run-integration-common-test

@js00070 js00070 requested a review from a team as a code owner October 14, 2019 09:33
@ghost ghost requested review from qw4990 and removed request for a team October 14, 2019 09:33
@Reminiscent Reminiscent removed their request for review October 14, 2019 11:40
@qw4990 qw4990 removed the request for review from SunRunAway October 16, 2019 09:01
@js00070 js00070 requested a review from qw4990 October 17, 2019 06:32
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, well done.

@qw4990 qw4990 removed their request for review October 22, 2019 08:42
Reminiscent
Reminiscent previously approved these changes Oct 22, 2019
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 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. status/ReqChange labels Oct 22, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Oct 22, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Oct 22, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Oct 22, 2019

/run-all-tests

@qw4990
Copy link
Contributor

qw4990 commented Oct 22, 2019

/build

@qw4990
Copy link
Contributor

qw4990 commented Oct 22, 2019

/rebuild

@qw4990
Copy link
Contributor

qw4990 commented Oct 22, 2019

@qrr1995 The CLA is blocked, please help us fix it.

@qw4990
Copy link
Contributor

qw4990 commented Oct 23, 2019

@js00070 Please resolve conflicts.

@qw4990
Copy link
Contributor

qw4990 commented Oct 23, 2019

/run-all-tests

@qw4990
Copy link
Contributor

qw4990 commented Oct 23, 2019

/run-unit-test

1 similar comment
@qw4990
Copy link
Contributor

qw4990 commented Oct 23, 2019

/run-unit-test

@qw4990 qw4990 merged commit c6d284e into pingcap:master Oct 23, 2019
@js00070 js00070 deleted the pr0 branch October 23, 2019 11:26
lfkdsk added a commit to JustProject/tidb that referenced this pull request Oct 26, 2019
…ect/tidb into feature-add-udf-support

* 'feature-add-udf-support' of https://github.com/JustProject/tidb: (26 commits)
  *: fix bug that the kill command doesn't work when the killed session is waiting for the pessimistic lock (pingcap#12852)
  executor: fix the projection upon the indexLookUp in indexLookUpJoin can't get result. (pingcap#12889)
  planner, executor: support create view on union (pingcap#12595)
  planner/cascades: introduce TransformationID in cascades planner (pingcap#12879)
  executor: fix data race in test (pingcap#12910)
  executor: reuse chunk row for insert on duplicate update (pingcap#12847)
  ddl: speed up tests (pingcap#12888)
  executor: speed up test (pingcap#12896)
  expression: implement vectorized evaluation for `builtinSecondSig` (pingcap#12886)
  expression: implement vectorized evaluation for `builtinJSONObjectSig` (pingcap#12663)
  expression: speed up builtinRepeatSig by using MergeNulls (pingcap#12674)
  expression: speed up unit tests under the expression package (pingcap#12887)
  store,kv: snapshot doesn't cache the non-exists kv entries lead to poor 'insert ignore' performance (pingcap#12872)
  executor: fix data race in `GetDirtyTable()` (pingcap#12767)
  domain: increase TTL to reduce the occurrence of reporting min startTS errors (pingcap#12578)
  executor: split test for speed up (pingcap#12881)
  executor: fix inconsistent of grants privileges with MySQL when executing `grant all on ...` (pingcap#12330)
  expression: implement vectorized evaluation for `builtinJSONUnquoteSig` (pingcap#12841)
  tune grpc connection count between tidb and tikv (pingcap#12884)
  Makefile: change test parallel to 8 (pingcap#12885)
  ...
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.

8 participants