Merge pull request #15 from huan086/master

Reduce number of calls to JsPool._metadata indexer to reduce synchron…

Author: Daniel15

src/JSPool.Tests/JsPoolTests.cs

@@ -21,7 +21,7 @@ public class JsPoolTests
 		public void ConstructorCreatesEngines()
 		{
 			var factory = new Mock<IEngineFactoryForMock>();
-			factory.Setup(x => x.EngineFactory()).Returns(new Mock<IJsEngine>().Object);
+			factory.Setup(x => x.EngineFactory()).Returns(() => new Mock<IJsEngine>().Object);
 			var config = new JsPoolConfig
 			{
 				StartEngines = 5,
@@ -72,7 +72,7 @@ public void GetEngineReturnsAllAvailableEngines()
 		public void GetEngineCreatesNewEngineIfNotAtMaximum()
 		{
 			var factory = new Mock<IEngineFactoryForMock>();
-			factory.Setup(x => x.EngineFactory()).Returns(new Mock<IJsEngine>().Object);
+			factory.Setup(x => x.EngineFactory()).Returns(() => new Mock<IJsEngine>().Object);
 			var config = new JsPoolConfig
 			{
 				StartEngines = 1,
@@ -97,7 +97,7 @@ public void GetEngineCreatesNewEngineIfNotAtMaximum()
 		public void GetEngineFailsIfAtMaximum()
 		{
 			var factory = new Mock<IEngineFactoryForMock>();
-			factory.Setup(x => x.EngineFactory()).Returns(new Mock<IJsEngine>().Object);
+			factory.Setup(x => x.EngineFactory()).Returns(() => new Mock<IJsEngine>().Object);
 			var config = new JsPoolConfig
 			{
 				StartEngines = 1,
@@ -109,7 +109,7 @@ public void GetEngineFailsIfAtMaximum()
 			factory.Verify(x => x.EngineFactory(), Times.Exactly(2));
 			pool.GetEngine(); // First engine created on init
 
-			Assert.Throws<JsPoolExhaustedException>(() => 
+			Assert.Throws<JsPoolExhaustedException>(() =>
 				pool.GetEngine(TimeSpan.Zero)
 			);
 		}
@@ -118,7 +118,7 @@ public void GetEngineFailsIfAtMaximum()
 		public void ReturnEngineToPoolAddsToAvailableEngines()
 		{
 			var factory = new Mock<IEngineFactoryForMock>();
-			factory.Setup(x => x.EngineFactory()).Returns(new Mock<IJsEngine>().Object);
+			factory.Setup(x => x.EngineFactory()).Returns(() => new Mock<IJsEngine>().Object);
 			var config = new JsPoolConfig
 			{
 				StartEngines = 2,
@@ -138,7 +138,7 @@ public void ReturnEngineDisposesIfAtMaxUsages()
 		{
 			var mockEngine1 = new Mock<IJsEngine>();
 			var mockEngine2 = new Mock<IJsEngine>();
-            var factory = new Mock<IEngineFactoryForMock>();
+			var factory = new Mock<IEngineFactoryForMock>();
 			factory.SetupSequence(x => x.EngineFactory())
 				// First engine is a dummy engine to check functionality
 				.Returns(new Mock<IJsEngine>().Object)
@@ -229,7 +229,7 @@ public void ShouldIgnoreReturnToPoolIfUnknownEngine()
 		public void RecycleCreatesNewEngines()
 		{
 			var factory = new Mock<IEngineFactoryForMock>();
-			factory.Setup(x => x.EngineFactory()).Returns(new Mock<IJsEngine>().Object);
+			factory.Setup(x => x.EngineFactory()).Returns(() => new Mock<IJsEngine>().Object);
 			var config = new JsPoolConfig
 			{
 				StartEngines = 2,
@@ -254,7 +254,7 @@ public void RecycleFiresRecycledEvent()
 		{
 			var callCount = 0;
 			var factory = new Mock<IEngineFactoryForMock>();
-			factory.Setup(x => x.EngineFactory()).Returns(new Mock<IJsEngine>().Object);
+			factory.Setup(x => x.EngineFactory()).Returns(() => new Mock<IJsEngine>().Object);
 			var config = new JsPoolConfig
 			{
 				StartEngines = 2,
@@ -276,7 +276,7 @@ public void RecycleFiresRecycledEvent()
 		public void WatchPathWithoutWatchFilesDoesNotThrow()
 		{
 			var factory = new Mock<IEngineFactoryForMock>();
-			factory.Setup(x => x.EngineFactory()).Returns(new Mock<IJsEngine>().Object);
+			factory.Setup(x => x.EngineFactory()).Returns(() => new Mock<IJsEngine>().Object);
 			var config = new JsPoolConfig
 			{
 				StartEngines = 2,
@@ -287,7 +287,7 @@ public void WatchPathWithoutWatchFilesDoesNotThrow()
 			{
 				// ReSharper disable once UnusedVariable
 				var pool = new JsPool(config);
-			});	
+			});
 		}
 	}
 

src/JSPool/JsPool.cs

@@ -30,14 +30,10 @@ public class JsPool : IJsPool
 		/// </summary>
 		protected readonly BlockingCollection<IJsEngine> _availableEngines = new BlockingCollection<IJsEngine>();
 		/// <summary>
-		/// Metadata for the engines.
+		/// Metadata for the engines. Total number of engines that have been created is reflected in its Count.
 		/// </summary>
 		protected readonly IDictionary<IJsEngine, EngineMetadata> _metadata = new ConcurrentDictionary<IJsEngine, EngineMetadata>();
 		/// <summary>
-		/// Totan number of engines that have been created.
-		/// </summary>
-		protected int _engineCount;
-		/// <summary>
 		/// Factory method used to create engines.
 		/// </summary>
 		protected readonly Func<IJsEngine> _engineFactory;
@@ -131,7 +127,6 @@ protected virtual IJsEngine CreateEngine()
 			var engine = _engineFactory();
 			_config.Initializer(engine);
 			_metadata[engine] = new EngineMetadata();
-			Interlocked.Increment(ref _engineCount);
 			return engine;
 		}
 
@@ -190,8 +185,9 @@ public virtual IJsEngine GetEngine(TimeSpan? timeout = null)
 		/// <param name="engine"></param>
 		private IJsEngine TakeEngine(IJsEngine engine)
 		{
-			_metadata[engine].InUse = true;
-			_metadata[engine].UsageCount++;
+			var metadata = _metadata[engine];
+			metadata.InUse = true;
+			metadata.UsageCount++;
 			return engine;
 		}
 
@@ -201,7 +197,8 @@ private IJsEngine TakeEngine(IJsEngine engine)
 		/// <param name="engine">Engine to return</param>
 		public virtual void ReturnEngineToPool(IJsEngine engine)
 		{
-			if (!_metadata.ContainsKey(engine))
+			EngineMetadata metadata;
+			if (!_metadata.TryGetValue(engine, out metadata))
 			{
 				// This engine was from another pool. This could happen if a pool is recycled
 				// and replaced with a different one (like what ReactJS.NET does when any 
@@ -210,17 +207,17 @@ public virtual void ReturnEngineToPool(IJsEngine engine)
 				return;
 			}
 
-			_metadata[engine].InUse = false;
-			var usageCount = _metadata[engine].UsageCount;
-            if (_config.MaxUsagesPerEngine > 0 && usageCount >= _config.MaxUsagesPerEngine)
+			metadata.InUse = false;
+			var usageCount = metadata.UsageCount;
+			if (_config.MaxUsagesPerEngine > 0 && usageCount >= _config.MaxUsagesPerEngine)
 			{
 				// Engine has been reused the maximum number of times, recycle it.
 				DisposeEngine(engine);
 				return;
 			}
 
 			if (
-				_config.GarbageCollectionInterval > 0 && 
+				_config.GarbageCollectionInterval > 0 &&
 				usageCount % _config.GarbageCollectionInterval == 0 &&
 				engine.SupportsGarbageCollection()
 			)
@@ -242,7 +239,6 @@ public virtual void DisposeEngine(IJsEngine engine, bool repopulateEngines = tru
 		{
 			engine.Dispose();
 			_metadata.Remove(engine);
-			Interlocked.Decrement(ref _engineCount);
 
 			if (repopulateEngines)
 			{
@@ -266,7 +262,6 @@ protected virtual void DisposeAllEngines()
 			// Also clear out all metadata so engines that are currently in use while this disposal is 
 			// happening get disposed on return.
 			_metadata.Clear();
-			_engineCount = 0;
 		}
 
 		/// <summary>
@@ -303,7 +298,7 @@ public virtual void Dispose()
 		/// </summary>
 		public virtual int EngineCount
 		{
-			get { return _engineCount; }
+			get { return _metadata.Count; }
 		}
 
 		/// <summary>