alastairtree / LazyCache

An easy to use thread safe in-memory caching service with a simple developer friendly API for c#
https://nuget.org/packages/LazyCache
MIT License
1.72k stars 159 forks source link

Double check lock in GetOrAdd #48

Closed MaximTkachenko closed 6 years ago

MaximTkachenko commented 6 years ago

What do you thin about using double check lock approach in CachingService.GetOrAdd like this: 1. try get value from cache 2. if key is missed aquire lock 3. get or create inside lock?

alastairtree commented 6 years ago

Have you looked at the dot net core version of the code? As the locking for that ended up being a bit more of a pain. Also you need to be aware that the async version of the methods will need a different implementation. Was there a particular issue/bug with the current implementation?

MaximTkachenko commented 6 years ago

I mean this one https://github.com/alastairtree/LazyCache/blob/feat/netcore2/LazyCache/CachingService.cs . I haven't faced with issues, just asking.

alastairtree commented 6 years ago

Yes that strategy may work but what you describe is not quite double check locking in the traditional sense. It would depend if the 1 Get and 1 occasional GetOrAdd call combination were more performant than the blanket locking approach used currently. also would need some thorough testing, and need to be aware of the locking across both sync and async versions of the method.

TBH I just followed the Jon Skeek advise below for ease!

Personally, if I'm in a situation where I have to choose between double-checked locking and "lock every time" code I'd go for locking every time until I'd got real evidence that it was causing a bottleneck. When it comes to threading, a simple and obviously-correct pattern is worth a lot.

https://stackoverflow.com/a/394932/3140853