Skip to content

executor: cache column info for prepare statement#12388

Merged
sre-bot merged 17 commits intopingcap:masterfrom
cfzjywxk:cacheColumnBytes
Sep 29, 2019
Merged

executor: cache column info for prepare statement#12388
sre-bot merged 17 commits intopingcap:masterfrom
cfzjywxk:cacheColumnBytes

Conversation

@cfzjywxk
Copy link
Contributor

@cfzjywxk cfzjywxk commented Sep 25, 2019 •
edited
Loading

What problem does this PR solve?

  • cache column info arrays for prepare statement to avoid repeatedly doing column info generation
  • cache column info bytes for prepare statement to improve performance

What is changed and how it works?

Check List

Tests

  • Unit test

Code changes

  • Has exported variable/fields change

Side effects

  • Increased code complexity

Related changes

Release note

@cfzjywxk
Copy link
Contributor Author

cfzjywxk commented Sep 25, 2019

/bench

@cfzjywxk
Copy link
Contributor Author

cfzjywxk commented Sep 25, 2019

/run-unit-test

coocood
coocood reviewed Sep 25, 2019
View reviewed changes
coocood
coocood reviewed Sep 25, 2019
View reviewed changes
@sre-bot
Copy link
Contributor

sre-bot commented Sep 25, 2019 

@@                               Benchmark Diff                               @@
================================================================================
--- tidb: 4686eca5858ff1228c325b2ed85a63c87f0a8210
+++ tidb: 99515b5291ceb599bf83706b67f7bd6d132153af
tikv: 9b59dee62d666d9d705b465383ff38081623bbe8
pd: c973515613157209832d943d698fc3e2133624a5
================================================================================
test-1: < oltp_read_write >
    * QPS : 37025.76 ± 0.0065% (std=1.73) delta: -0.45% (p=0.206)
    * AvgMs : 138.61 ± 0.3939% (std=0.34) delta: 0.37%
    * PercentileMs99 : 269.35 ± 1.0826% (std=2.38) delta: 0.00%
            
test-2: < oltp_point_select >
    * QPS : 81532.21 ± 1.6810% (std=810.31) delta: -0.96% (p=0.358)
    * AvgMs : 3.14 ± 1.6571% (std=0.03) delta: 0.84%
    * PercentileMs99 : 6.77 ± 2.1283% (std=0.09) delta: 1.44%
            
test-3: < oltp_insert >
    * QPS : 21795.83 ± 0.3242% (std=40.93) delta: -0.51% (p=0.521)
    * AvgMs : 11.74 ± 0.3195% (std=0.02) delta: 0.49%
    * PercentileMs99 : 23.95 ± 0.0000% (std=0.00) delta: 0.36%
            
test-4: < oltp_update_index >
    * QPS : 17046.31 ± 1.1036% (std=154.08) delta: 0.18% (p=0.818)
    * AvgMs : 14.87 ± 0.4169% (std=0.04) delta: -0.04%
    * PercentileMs99 : 30.81 ± 0.0000% (std=0.00) delta: 0.00%
            
test-5: < oltp_update_non_index >
    * QPS : 29130.67 ± 0.4701% (std=81.36) delta: -0.10% (p=0.680)
    * AvgMs : 8.80 ± 0.2558% (std=0.01) delta: 0.31%
    * PercentileMs99 : 19.09 ± 1.0688% (std=0.17) delta: 0.72%

https://perf.pingcap.com

@codecov
Copy link

codecov bot commented Sep 25, 2019 •
edited
Loading

Codecov Report

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

@@             Coverage Diff             @@
##             master     #12388   +/-   ##
===========================================
  Coverage   80.0866%   80.0866%           
===========================================
  Files           462        462           
  Lines        103644     103644           
===========================================
  Hits          83005      83005           
  Misses        14727      14727           
  Partials       5912       5912

coocood
coocood reviewed Sep 25, 2019
View reviewed changes
coocood
coocood reviewed Sep 25, 2019
View reviewed changes
coocood
coocood reviewed Sep 25, 2019
View reviewed changes
coocood
coocood reviewed Sep 25, 2019
View reviewed changes
@cfzjywxk
Copy link
Contributor Author

cfzjywxk commented Sep 25, 2019

rs.Columns() is an expensive call, try to skip this usage

@cfzjywxk
Copy link
Contributor Author

cfzjywxk commented Sep 25, 2019

/bench

coocood
coocood reviewed Sep 25, 2019
View reviewed changes
@cfzjywxk cfzjywxk changed the title executor: cache column info bytes for prepare statement executor: cache column info for prepare statement Sep 25, 2019
@cfzjywxk
Copy link
Contributor Author

cfzjywxk commented Sep 25, 2019

/run-unit-test

@coocood
Copy link
Member

coocood commented Sep 25, 2019

@cfzjywxk
We don't need to store ColumnInfo for text protocol, text prepared statement it's rarely used.

@cfzjywxk
Copy link
Contributor Author

cfzjywxk commented Sep 25, 2019

/bench

@cfzjywxk
Copy link
Contributor Author

cfzjywxk commented Sep 25, 2019 •
edited
Loading

@cfzjywxk
We don't need to store ColumnInfo for text protocol, text prepared statement it's rarely used.

prepareStatement is only saved in TiDBStatement.Execute which will only be used by binary protocol
@coocood

@cfzjywxk
Copy link
Contributor Author

cfzjywxk commented Sep 26, 2019

/run-unit-test

@cfzjywxk
Copy link
Contributor Author

cfzjywxk commented Sep 26, 2019

/bench

@coocood
Copy link
Member

coocood commented Sep 26, 2019

LGTM

@cfzjywxk cfzjywxk requested review from jackysp and lysu September 26, 2019 07:54
@cfzjywxk cfzjywxk added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 26, 2019
@cfzjywxk
Copy link
Contributor Author

cfzjywxk commented Sep 26, 2019

@jackysp @lysu PTAL

@sre-bot
Copy link
Contributor

sre-bot commented Sep 26, 2019 

@@                               Benchmark Diff                               @@
================================================================================
--- tidb: e41458983c4cf436ad46768a5c9d5b79551c8a3e
+++ tidb: 46b0af6ac59557875e7b95592c68562d4a073479
tikv: 31f3ee31fd82e291e3ce842ca231ba103e9a5e89
pd: c973515613157209832d943d698fc3e2133624a5
================================================================================
test-1: < oltp_read_write >
    * QPS : 37070.72 ± 0.1518% (std=32.92) delta: -0.99% (p=0.009)
    * AvgMs : 138.40 ± 0.4827% (std=0.48) delta: 0.82%
    * PercentileMs99 : 267.41 ± 0.0000% (std=0.00) delta: 0.00%
            
test-2: < oltp_point_select >
    * QPS : 82326.82 ± 1.6880% (std=905.91) delta: 1.08% (p=0.081)
    * AvgMs : 3.11 ± 1.6077% (std=0.03) delta: -1.03%
    * PercentileMs99 : 6.62 ± 2.5370% (std=0.10) delta: -1.78%
            
test-3: < oltp_insert >
    * QPS : 21915.24 ± 0.2259% (std=40.95) delta: 0.17% (p=0.810)
    * AvgMs : 11.68 ± 0.2141% (std=0.02) delta: -0.18%
    * PercentileMs99 : 23.78 ± 2.5318% (std=0.34) delta: -0.72%
            
test-4: < oltp_update_index >
    * QPS : 16995.45 ± 1.6955% (std=202.92) delta: -0.63% (p=0.392)
    * AvgMs : 14.89 ± 0.7121% (std=0.08) delta: 0.00%
    * PercentileMs99 : 30.81 ± 0.0000% (std=0.00) delta: 0.00%
            
test-5: < oltp_update_non_index >
    * QPS : 29053.36 ± 0.0268% (std=6.16) delta: -0.40% (p=0.082)
    * AvgMs : 8.81 ± 0.0000% (std=0.00) delta: 0.43%
    * PercentileMs99 : 19.29 ± 0.0000% (std=0.00) delta: 1.79%

https://perf.pingcap.com

@sre-bot
Copy link
Contributor

sre-bot commented Sep 28, 2019

@coocood, @lysu, @jackysp, PTAL.

lysu
lysu reviewed Sep 29, 2019
View reviewed changes
Copy link
Contributor

@lysu lysu left a comment

Choose a reason for hiding this comment

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

rest LGTM

@lysu lysu added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. status/PTAL labels Sep 29, 2019
@coocood coocood added the status/can-merge Indicates a PR has been approved by a committer. label Sep 29, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Sep 29, 2019

/run-all-tests

@sre-bot sre-bot merged commit 2d3dfac into pingcap:master Sep 29, 2019
This was referenced Mar 28, 2022
Closed
Merged
This was referenced Mar 30, 2022
Closed
Closed
Closed
Closed
Closed
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coocood coocood coocood approved these changes

@jackysp jackysp Awaiting requested review from jackysp

+1 more reviewer

@lysu lysu lysu left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/performance

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cfzjywxk @sre-bot @coocood @lysu