-
Notifications
You must be signed in to change notification settings - Fork 31
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
perf: apply rw mutex to cachekv #204
Conversation
8dc9ffa
to
6113434
Compare
6113434
to
e901359
Compare
Codecov Report
@@ Coverage Diff @@
## v2/develop #204 +/- ##
==============================================
+ Coverage 53.48% 53.49% +0.01%
==============================================
Files 652 652
Lines 47341 47344 +3
==============================================
+ Hits 25318 25325 +7
+ Misses 19170 19166 -4
Partials 2853 2853
|
e901359
to
163bb34
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think sync.Map is necessary as it's accessed with lock held.
@iproudhon I think |
I think we should separate the read-only cache from the |
Ah, it checks whether the item is dirty. |
Yes, I think |
As it has over 66% update, sync.Map is the right choice. With that, I don't think we need RWLock. Instead, we can just use Mutex to protect setCacheValue, Write and iterator. Get() doesn't need lock protection. |
@iproudhon When |
Ok, I got it. |
Description
When checkTx is executed concurrently, lock contention occurs on
Read
incachekv.Store
.So modify
cachekv.Store
to useRWLock
.Also, since set
cache
inRead
, concurrent-safecache
is required. So I usesync.Map
.-->
closes: https://github.com/line/lbm/issues/1187
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.
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passes