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

chore: caching paramset #198

Merged
merged 9 commits into from
May 21, 2021
Merged

Conversation

egonspace
Copy link

@egonspace egonspace commented May 18, 2021

Description

We found that cachekv.mutex had a performance degradation.
We need to replace this mutex with RWMutex, but by caching the object in ParamSet before that, we can reduce the mutex contention that occurs frequently in cachekv.Get() calls.

I measured performance improvements of 1845 -> 1895 tps based on single remote node after this modification.

closes: #XXXX


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

@egonspace egonspace self-assigned this May 18, 2021
@egonspace egonspace added this to the performance improvement milestone May 18, 2021
@codecov
Copy link

codecov bot commented May 18, 2021

Codecov Report

Merging #198 (0c78434) into v2/develop (623dd8c) will increase coverage by 0.05%.
The diff coverage is 76.19%.

Impacted file tree graph

@@              Coverage Diff               @@
##           v2/develop     #198      +/-   ##
==============================================
+ Coverage       53.46%   53.51%   +0.05%     
==============================================
  Files             653      653              
  Lines           47289    47384      +95     
==============================================
+ Hits            25284    25359      +75     
- Misses          19155    19170      +15     
- Partials         2850     2855       +5     
Impacted Files Coverage Δ
simapp/app.go 83.79% <ø> (ø)
x/auth/keeper/keeper.go 48.61% <ø> (ø)
x/bank/keeper/keeper.go 75.00% <ø> (ø)
x/bank/keeper/send.go 85.08% <ø> (ø)
x/crisis/keeper/keeper.go 86.20% <ø> (ø)
x/distribution/keeper/keeper.go 81.57% <ø> (ø)
x/ibc/applications/transfer/keeper/keeper.go 83.33% <ø> (ø)
x/ibc/core/02-client/keeper/keeper.go 83.64% <ø> (ø)
x/ibc/core/keeper/keeper.go 81.25% <ø> (ø)
x/mint/keeper/keeper.go 64.51% <ø> (ø)
... and 18 more

@egonspace egonspace added the WIP label May 18, 2021
@egonspace egonspace removed the WIP label May 20, 2021
@egonspace egonspace removed the request for review from kukugi May 21, 2021 02:12
Copy link
Contributor

@wetcod wetcod left a comment

Choose a reason for hiding this comment

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

LGTM

@whylee259
Copy link
Contributor

Looks nice!

@egonspace egonspace merged commit dc15595 into v2/develop May 21, 2021
@egonspace egonspace deleted the egon/v2/perf/caching_paramset branch May 21, 2021 06:28
@egonspace egonspace mentioned this pull request Sep 24, 2021
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants