Switched to AsyncKeyedLock #3066
Conversation
| { | ||
| lock (urlcachelock) | ||
| lock (urlcacheSem) | ||
| if (urlcache.ContainsKey(url.AbsoluteUri)) |
There was a problem hiding this comment.
is this guaranteed to contain the key now? given the above using.
i understand that your the author of the library?
the intent of this function was to dedup http requests/cache and respond
There was a problem hiding this comment.
I'm not sure I follow what you mean in your first sentence.
Yes, I am the author of the AsyncKeyedLock library.
There was a problem hiding this comment.
line
using (urlcacheSem.Lock(url.AbsoluteUri))
must create a state
that
if (urlcache.ContainsKey(url.AbsoluteUri))
will not fail on? ie is it redundant?
There was a problem hiding this comment.
No I don't think so. Imagine you fire up the application, urlcache and urlcacheSem will both be empty. If you make a new call to a URL, urlcacheSem will be used, obtains the lock and goes check urlcache to see if it's in the cache. It won't find it.
|
Is there anything else you need @meee1 ? |
|
@meee1 are you interested or shall I close this PR? |
|
@meee1 please let me know |
This helps clean up from the dictionary and reduces memory allocations. However I'm not sure if
urlcacheSemshould be static. It feels like it should, but the existing code keeps the dictionaries separate.