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

*: use int instead of fmt.Stringer as executor id #19207

Merged
merged 21 commits into from
Aug 19, 2020

Conversation

crazycs520
Copy link
Contributor

What problem does this PR solve?

fix #18899

  • Use int (plan id) instead of fmt.Stringer as executor id

image

  • Add SearchTrackerWithoutLock for Tracker to avoid needless lock:

image

benchmark

For big-quey like union with 10000 select statement:

        // generate SQL
	buf := bytes.NewBuffer(make([]byte, 0, 1024*1024*4))
	for i := 0; i < 10000; i++ {
		if i > 0 {
			buf.WriteString(" union ")
		}
		buf.WriteString(fmt.Sprintf("select count(1) as num,a from t where a='%v' group by a", i))
	}
	query := buf.String()

benchmark the core.EncodePlan function, this pr almost 25 times faster than before.

// before this PR:
12.93s

// This PR:
0.516s

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • No logical Code.

Side effects

  • No

Release note

  • Optimize the performance of encoding plan.

@crazycs520 crazycs520 added type/enhancement The issue or PR belongs to an enhancement. component/executor labels Aug 14, 2020
@crazycs520 crazycs520 requested review from a team as code owners August 14, 2020 09:07
@crazycs520 crazycs520 requested review from XuHuaiyu and removed request for a team August 14, 2020 09:07
distsql/select_result.go Show resolved Hide resolved
executor/index_lookup_merge_join.go Outdated Show resolved Hide resolved
executor/index_merge_reader.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Aug 14, 2020

Codecov Report

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

@@             Coverage Diff             @@
##             master     #19207   +/-   ##
===========================================
  Coverage   79.4944%   79.4944%           
===========================================
  Files           549        549           
  Lines        152432     152432           
===========================================
  Hits         121175     121175           
  Misses        21724      21724           
  Partials       9533       9533           

@crazycs520
Copy link
Contributor Author

/bench

@sre-bot
Copy link
Contributor

sre-bot commented Aug 14, 2020

Benchmark Report

Run Sysbench Performance Test on VMs

@@                               Benchmark Diff                               @@
================================================================================
--- tidb: a4d26bd57c8cd3bf7605912cec89fc1a538ebc88
+++ tidb: e14b57c6bc1521827db7fb55014542cebf81226f
tikv: ae7a6ecee6e3367da016df0293a9ffe9cc2b5705
pd: ec99270dfe418971fd8c62b87d23296fffa12265
================================================================================
oltp_update_index:
    * QPS: 16632.75 ± 0.43% (std=54.09) delta: -0.21% (p=0.458)
    * Latency p50: 7.69 ± 0.45% (std=0.03) delta: 0.24%
    * Latency p99: 15.55 ± 0.00% (std=0.00) delta: 0.91%
            
oltp_insert:
    * QPS: 20777.02 ± 0.18% (std=27.78) delta: 1.57% (p=0.017)
    * Latency p50: 6.16 ± 0.16% (std=0.01) delta: -1.48%
    * Latency p99: 12.52 ± 0.00% (std=0.00) delta: -1.80%
            
oltp_update_non_index:
    * QPS: 20978.87 ± 0.41% (std=61.64) delta: -0.15% (p=0.446)
    * Latency p50: 6.10 ± 0.45% (std=0.02) delta: 0.12%
    * Latency p99: 12.75 ± 0.00% (std=0.00) delta: 0.00%
            
oltp_read_write:
    * QPS: 30995.52 ± 0.22% (std=46.45) delta: 0.05% (p=0.678)
    * Latency p50: 83.53 ± 0.23% (std=0.14) delta: -0.04%
    * Latency p99: 148.95 ± 0.90% (std=1.34) delta: 0.91%
            
oltp_point_select:
    * QPS: 66020.60 ± 0.85% (std=377.42) delta: -0.95% (p=0.069)
    * Latency p50: 1.94 ± 0.34% (std=0.00) delta: 1.22%
    * Latency p99: 8.90 ± 0.00% (std=0.00) delta: 3.73%
            

Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

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

lgtm

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 18, 2020
@qw4990 qw4990 self-requested a review August 19, 2020 02:46
qw4990
qw4990 previously approved these changes Aug 19, 2020
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

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Aug 19, 2020
ti-srebot
ti-srebot previously approved these changes Aug 19, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Aug 19, 2020
@crazycs520 crazycs520 dismissed stale reviews from ti-srebot and qw4990 via f41d5ef August 19, 2020 02:54
@crazycs520
Copy link
Contributor Author

/run-all-tests

@crazycs520
Copy link
Contributor Author

/run-unit-test

@crazycs520
Copy link
Contributor Author

/run-all-tests

@crazycs520
Copy link
Contributor Author

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

@crazycs520
Copy link
Contributor Author

/run-integration-common-test

@qw4990 qw4990 merged commit a2e2ce6 into pingcap:master Aug 19, 2020
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Aug 19, 2020
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #19279

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/executor status/LGT2 Indicates that a PR has LGTM 2. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimize the performance for getRuntimeInfo of big query
5 participants