Skip to content

expression: implement vectorized evaluation for builtinJSONUnquoteSig#12841

Merged
ngaut merged 6 commits intopingcap:masterfrom
js00070:JsonUnquote
Oct 23, 2019
Merged

expression: implement vectorized evaluation for builtinJSONUnquoteSig#12841
ngaut merged 6 commits intopingcap:masterfrom
js00070:JsonUnquote

Conversation

@js00070
Copy link
Contributor

@js00070 js00070 commented Oct 20, 2019

What problem does this PR solve?

implement vectorized evaluation for builtinJSONUnquoteSig mentioned at #12104

What is changed and how it works?

about 10% faster

BenchmarkVectorizedBuiltinJSONFunc/builtinJSONUnquoteSig-VecBuiltinFunc-12                  6319            196491 ns/op           65680 B/op       1642 allocs/op
BenchmarkVectorizedBuiltinJSONFunc/builtinJSONUnquoteSig-NonVecBuiltinFunc-12               5443            218511 ns/op           65680 B/op       1642 allocs/op

Check List

Tests

  • Unit test

@js00070 js00070 requested a review from a team as a code owner October 20, 2019 16:26
@ghost ghost requested review from XuHuaiyu and removed request for a team October 20, 2019 16:26
@sre-bot sre-bot added the contribution This PR is from a community contributor. label Oct 20, 2019
@codecov
Copy link

codecov bot commented Oct 20, 2019

Codecov Report

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

@@             Coverage Diff             @@
##             master     #12841   +/-   ##
===========================================
  Coverage   80.1359%   80.1359%           
===========================================
  Files           465        465           
  Lines        107692     107692           
===========================================
  Hits          86300      86300           
  Misses        14927      14927           
  Partials       6465       6465

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
Copy link
Contributor

qw4990 commented Oct 21, 2019

@js00070 Please solve conflicts.

@js00070
Copy link
Contributor Author

js00070 commented Oct 21, 2019

@js00070 Please solve conflicts.

conflicts have been solved

ast.JSONMerge: {},
ast.JSONInsert: {},
ast.JSONUnquote: {
{retEvalType: types.ETString, childrenTypes: []types.EvalType{types.ETJson}},
Copy link
Contributor

Choose a reason for hiding this comment

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

The string generated by default generator will never be a JSON, consider using jsonStringGener in bench_test.go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I have used JsonStringGener in test case now.

@qw4990 qw4990 added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 22, 2019
@qw4990 qw4990 requested a review from SunRunAway October 22, 2019 09:23
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. 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

@js00070 merge failed.

@ngaut ngaut merged commit a9c92e4 into pingcap:master Oct 23, 2019
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)
  ...
@js00070 js00070 deleted the JsonUnquote branch November 7, 2019 11:08
XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019
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.

7 participants