Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Improve performance of MemoryCache by using ConcurrentDictionary rather than ReaderWriterLockSlim #242

Closed
mikeharder opened this issue Oct 20, 2016 · 4 comments

Comments

@mikeharder
Copy link
Contributor

It may be possible to significantly improve the performance of MemoryCache by using ConcurrentDictionary rather than Dictionary protected by a ReaderWriterLockSlim. Under high load, there is contention acquiring the read lock from ReaderWriterLockSlim.

I created a quick prototype which shows significantly improved performance, though it may not be functionally correct in all scenarios.

Scenario MemoryCache Implementation RPS CPU
Plaintext N/A 1,167,369 100%
MemoryCachePlaintext ReaderWriterLockSlim 342,129 60%
MemoryCachePlaintext ConcurrentDictionary 802,076 100%

The remaining work is to improve and test the ConcurrentDictionary implementation to ensure it's correct under high load, has no race conditions, etc.

Prototype: https://github.com/aspnet/Caching/tree/mikeharder/concurrent-dictionary

@JunTaoLuo JunTaoLuo self-assigned this Oct 27, 2016
@muratg muratg added this to the 1.1.0 milestone Nov 3, 2016
@muratg muratg added the bug label Nov 3, 2016
@muratg muratg modified the milestones: 1.2.0, 1.1.0 Nov 3, 2016
@muratg muratg added enhancement and removed bug labels Nov 3, 2016
@Dissimilis
Copy link

Please thoroughly test ConcurrentDictionary vs lock'ed Dictionary performance and don't make this change blindly. Performance may depend a lot on specific workload (amount of get vs set, etc.), even simple Monitor may be faster than ReaderWriterLockSlim if most operations are writes.

@JunTaoLuo
Copy link
Contributor

@Dissimilis thanks for the feedback but is there any particular workload you are concerned about? A few scenarios we have benchmarked include 1) concurrent gets with a few set to update the entry and 2) concurrent sets and removes and have seen major improvements in both. See the referenced PR for benchmark results. The move toward ConcurrentDictionary also takes advantage of the more fine-grained locking and will significantly improve performance when concurrently accessing different entries in the cache compared to the previous implementation which uses a single ReaderWriterLockSlim across the entire dictionary.

@sebastienros
Copy link
Member

Can we update the table with the new numbers?

@JunTaoLuo
Copy link
Contributor

Scenario previous CPU previous RPS CPU RPS
benchmarks: memorycacheplaintext 60 342,129 100 886,544
benchmarks: memorycacheplaintextsetremove - - 100 689,634

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants