Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Lockless read of MongoUrl cache #118

Closed
wants to merge 3 commits into
from
Jump to file or symbol
Failed to load files and symbols.
+105 −19
Split
View
@@ -15,12 +15,8 @@
using System;
using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Text.RegularExpressions;
using MongoDB.Bson;
-using MongoDB.Driver.Internal;
namespace MongoDB.Driver
{
@@ -48,6 +44,7 @@ public class MongoUrl : IEquatable<MongoUrl>
{
// private static fields
private static object __staticLock = new object();
+
private static Dictionary<string, MongoUrl> __cache = new Dictionary<string, MongoUrl>();
// private fields
@@ -281,7 +278,11 @@ public TimeSpan WaitQueueTimeout
/// </summary>
public static void ClearCache()
{
- __cache.Clear();
+ // Replace cache with new empty one. GC will remove old cache when not referenced.
+ lock(__staticLock)
+ {
+ __cache = new Dictionary<string, MongoUrl>();
+ }
}
/// <summary>
@@ -292,28 +293,43 @@ public static void ClearCache()
public static MongoUrl Create(string url)
{
// cache previously seen urls to avoid repeated parsing
- lock (__staticLock)
+ MongoUrl mongoUrl;
+ // get ref to current dictionary instance.
+ var cacheRef = __cache;
+
+ if (!cacheRef.TryGetValue(url, out mongoUrl))
{
- MongoUrl mongoUrl;
- if (!__cache.TryGetValue(url, out mongoUrl))
+ lock (__staticLock)
{
- mongoUrl = new MongoUrl(url);
- var canonicalUrl = mongoUrl.ToString();
- if (canonicalUrl != url)
+ // re-grab the current cache ref. We got lock, but another thread may have modified before we entered lock.
+ cacheRef = __cache;
+ if (!cacheRef.TryGetValue(url, out mongoUrl))
{
- if (__cache.ContainsKey(canonicalUrl))
+ mongoUrl = new MongoUrl(url);
+ var canonicalUrl = mongoUrl.ToString();
+ if (canonicalUrl != url)
{
- mongoUrl = __cache[canonicalUrl]; // use existing MongoUrl
+ if (cacheRef.ContainsKey(canonicalUrl))
+ {
+ mongoUrl = cacheRef[canonicalUrl]; // use existing MongoUrl
+ }
+ else
+ {
+ cacheRef[canonicalUrl] = mongoUrl; // cache under canonicalUrl also
+ }
}
- else
+ // copy old dictionary values to new one. Can't guarantee concurrent write while unlocked reader may be accessing cachRef
+ Dictionary<string, MongoUrl> updatedCache = new Dictionary<string, MongoUrl>(cacheRef.Count+1);
+ updatedCache[url] = mongoUrl;
+ foreach (var entry in cacheRef)
{
- __cache[canonicalUrl] = mongoUrl; // cache under canonicalUrl also
+ updatedCache[entry.Key] = entry.Value;
}
+ __cache = updatedCache;
}
- __cache[url] = mongoUrl;
}
- return mongoUrl;
}
+ return mongoUrl;
}
// public methods
@@ -16,7 +16,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
-using System.Text;
+using System.Reflection;
using NUnit.Framework;
using MongoDB.Bson;
@@ -478,5 +478,75 @@ public void TestEquals()
Assert.IsFalse(n != null);
Assert.IsFalse(null != n);
}
+
+ [Test]
+ public void TestCacheEmptyWhenInstanceConstructed()
+ {
+ var target = new MongoUrl("mongodb://host1");
+
+ Dictionary<string, MongoUrl> cache = getCacheReference(target);
+
+ Assert.IsNotNull(cache);
+ Assert.AreEqual(0, cache.Count);
+ }
+
+ [Test]
+ public void TestCacheHasItemAfterCreate()
+ {
+ var target = MongoUrl.Create("mongodb://host1");
+
+ Dictionary<string, MongoUrl> cache = getCacheReference(target);
+
+ Assert.IsNotNull(cache);
+ Assert.AreEqual(1, cache.Count);
+ }
+
+ [Test]
+ public void TestCacheEmptyAfterClear()
+ {
+ var target = MongoUrl.Create("mongodb://host1");
+
+ Dictionary<string, MongoUrl> cache = getCacheReference(target);
+
+ Assert.AreEqual(1, cache.Count);
+ MongoUrl.ClearCache();
+
+ cache = getCacheReference(target);
+
+ Assert.AreEqual(0, cache.Count);
+ }
+
+
+ [Test]
+ public void TestCacheSameInstanceAcrossMultipleCreate()
+ {
+ var t1 = MongoUrl.Create("mongodb://host1");
+ var cache1 = getCacheReference(t1);
+ var t2 = MongoUrl.Create("mongodb://host2");
+ var cache2 = getCacheReference(t2);
+
+ Assert.AreNotSame(cache1, cache2, "Cache internal dictionary is replaced upon insertion to allow lockless reads");
+ Assert.AreEqual(1, cache1.Count, "Cache dictionary should have 1 item after the first insertion");
+ Assert.AreEqual(2, cache2.Count, "Cache dictionary shoudl have 2 items after the second insertion");
+ }
+
+ [Test]
+ public void TestCacheDifferentInstanceOnceClearCache()
+ {
+ var target = MongoUrl.Create("mongodb://host1");
+ var cache1 = getCacheReference(target);
+
+ MongoUrl.ClearCache();
+
+ var cache2 = getCacheReference(target);
+ Assert.AreNotSame(cache1, cache2, "The 2 cache references shoudl refer to different object since CacheClear() exchanges the old ref to a new empty dictionary");
+ }
+
+
+ private Dictionary<string, MongoUrl> getCacheReference(MongoUrl target)
+ {
+ var fieldInfo = typeof(MongoUrl).GetField("__cache", BindingFlags.Static | BindingFlags.GetField | BindingFlags.NonPublic);
+ return fieldInfo.GetValue(target) as Dictionary<string, MongoUrl>;
+ }
}
-}
+}