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.

Sign up for GitHub

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

Jump to bottom

controller: try use atomic replace the lock #8968

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

controller: try use atomic replace the lock #8968

wants to merge 1 commit into from
+145 −2

Conversation

nolouch
Copy link
Contributor

@nolouch nolouch commented Jan 3, 2025

What problem does this PR solve?

Issue Number: Close #8918

What is changed and how does it work?

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Code changes

Side effects

  • Possible performance regression
  • Increased code complexity
  • Breaking backward compatibility

Related changes

Release note

None.

@ti-chi-bot ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Jan 3, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@ti-chi-bot ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Indicates the PR's author has signed the dco. labels Jan 3, 2025
@ti-chi-bot ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Jan 3, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from nolouch, ensuring that each of them provides their approval before proceeding. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 3, 2025
@nolouch
Copy link
Contributor Author

nolouch commented Jan 3, 2025

The benchmark shows that the atomic version does not perform better than the lock version:

  • Local MPB (CPU(s): 12):
controller git:(optimize-locks) go test -run=^$ -benchmem -bench ^BenchmarkRequestAndResponseConsumption ./ -count=3 --tags=intest -cpu=2,4,8,16,1024,2048 -v
goos: darwin
goarch: arm64
pkg: github.com/tikv/pd/client/resource_group/controller
cpu: Apple M2 Max
BenchmarkRequestAndResponseConsumptionLockVer
BenchmarkRequestAndResponseConsumptionLockVer-2                 12052750               100.9 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-2                 12201801               100.8 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-2                 11879060                99.62 ns/op          128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-4                  8846917               138.1 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-4                  8755874               138.1 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-4                  8746006               136.2 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-8                  6519382               175.4 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-8                  6812086               172.5 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-8                  6747466               178.2 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-16                 5521766               195.8 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-16                 5893826               195.3 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-16                 6167022               196.3 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-1024               4183626               279.9 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-1024               4416201               258.6 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-1024               4511895               270.3 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-2048               4261791               284.3 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-2048               3896812               277.4 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-2048               4593768               274.1 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer
BenchmarkRequestAndResponseConsumptionAtomicVer-2               18196218                64.43 ns/op          128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-2               17640043                61.28 ns/op          128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-2               18291357                62.72 ns/op          128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-4               10625161               112.5 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-4               10201930                99.93 ns/op          128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-4               10800978               110.4 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-8                3272788               365.4 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-8                3302931               370.1 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-8                3450266               320.7 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-16               1738362              1126 ns/op             128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-16               1000000              1134 ns/op             128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-16               1750414              1099 ns/op             128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-1024             1000000              1016 ns/op             128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-1024             1000000              1085 ns/op             128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-1024             1000000              1076 ns/op             128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-2048             1000000              1068 ns/op             128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-2048             1878140               645.7 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-2048             1763342              1088 ns/op             129 B/op          2 allocs/op
PASS
  • cpu: Intel(R) Xeon(R) Gold 6240 CPU @ 2.60GHz. (CPU(s): 72)
➜  controller git:(optimize-locks)  go test -run=^$ -benchmem -bench ^BenchmarkRequestAndResponseConsumption ./ -count=3 --tags=intest -cpu=2,4,8,16,1024,2048 -v
goos: linux
goarch: amd64
pkg: github.com/tikv/pd/client/resource_group/controller
cpu: Intel(R) Xeon(R) Gold 6240 CPU @ 2.60GHz
BenchmarkRequestAndResponseConsumptionLockVer
BenchmarkRequestAndResponseConsumptionLockVer-2                  8621672               133.5 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-2                  8597760               133.8 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-2                  8630604               134.1 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-4                  7042933               167.9 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-4                  6869895               165.9 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-4                  7071890               167.5 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-8                  5608933               213.3 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-8                  5561970               228.7 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-8                  5012782               241.9 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-16                 3368307               398.2 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-16                 2986978               389.7 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-16                 4168164               316.8 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-1024               3739953               412.6 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-1024               3014074               391.3 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-1024               3042794               355.9 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-2048               3114734               425.2 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-2048               3019845               443.8 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionLockVer-2048               3350582               350.5 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer
BenchmarkRequestAndResponseConsumptionAtomicVer-2                3692481               302.4 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-2                4055090               372.3 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-2                4726200               250.8 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-4                3653085               321.7 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-4                3776768               337.6 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-4                3859587               311.1 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-8                3380364               693.6 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-8                2895282               433.0 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-8                2670873               389.0 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-16               1639359               732.7 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-16               2004878               546.0 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-16               3183615               739.7 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-1024             1214854               935.7 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-1024             1540971               983.6 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-1024             1538190               776.2 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-2048             1567233               921.2 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-2048             1218561               975.5 ns/op           128 B/op          2 allocs/op
BenchmarkRequestAndResponseConsumptionAtomicVer-2048             1566546               760.2 ns/op           128 B/op          2 allocs/op
PASS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
dco-signoff: yes Indicates the PR's author has signed the dco. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Optimize the lock usage in resource controller
1 participant
@nolouch