Always execute Babel with non-pooled JS Engine

Babel uses a LOT of RAM, so let's just keep it totally separate from the pool for the moment. Potentially this will improve if we use V8 contexts rather than totally separate engines.

Author: Daniel15

src/React.Core/Babel.cs

@@ -243,10 +243,9 @@ protected virtual JavaScriptWithSourceMap TransformWithHeader(
 		/// <returns>JavaScript</returns>
 		public virtual string Transform(string input, string filename = "unknown")
 		{
-			_environment.EnsureBabelLoaded();
 			try
 			{
-				var output = _environment.ExecuteWithLargerStackIfRequired<string>(
+				var output = _environment.ExecuteWithBabel<string>(
 					"ReactNET_transform",
 					input,
 					_babelConfig,
@@ -272,10 +271,9 @@ public virtual JavaScriptWithSourceMap TransformWithSourceMap(
 			string filename = "unknown"
 		)
 		{
-			_environment.EnsureBabelLoaded();
 			try
 			{
-				return _environment.ExecuteWithLargerStackIfRequired<JavaScriptWithSourceMap>(
+				return _environment.ExecuteWithBabel<JavaScriptWithSourceMap>(
 					"ReactNET_transform_sourcemap",
 					input,
 					_babelConfig,

src/React.Core/IReactEnvironment.cs

@@ -51,8 +51,11 @@ public interface IReactEnvironment
 		T Execute<T>(string function, params object[] args);
 
 		/// <summary>
-		/// Attempts to execute the provided JavaScript code using the current engine. If an 
-		/// exception is thrown, retries the execution using a new thread (and hence a new engine)
+		/// Attempts to execute the provided JavaScript code using a non-pooled JavaScript engine (ie.
+		/// creates a new JS engine per-thread). This is because Babel uses a LOT of memory, so we 
+		/// should completely dispose any engines that have loaded Babel in order to conserve memory.
+		/// 
+		/// If an exception is thrown, retries the execution using a new thread (and hence a new engine)
 		/// with a larger maximum stack size.
 		/// This is required because JSXTransformer uses a huge stack which ends up being larger 
 		/// than what ASP.NET allows by default (256 KB).
@@ -61,7 +64,7 @@ public interface IReactEnvironment
 		/// <param name="function">JavaScript function to execute</param>
 		/// <param name="args">Arguments to pass to function</param>
 		/// <returns>Result returned from JavaScript code</returns>
-		T ExecuteWithLargerStackIfRequired<T>(string function, params object[] args);
+		T ExecuteWithBabel<T>(string function, params object[] args);
 
 		/// <summary>
 		/// Determines if the specified variable exists in the JavaScript engine
@@ -91,10 +94,5 @@ public interface IReactEnvironment
 		/// Gets the JSX Transformer for this environment.
 		/// </summary>
 		IBabel Babel { get; }
-
-		/// <summary>
-		/// Ensures that Babel has been loaded into the JavaScript engine.
-		/// </summary>
-		void EnsureBabelLoaded();
 	}
 }

src/React.Core/ReactEnvironment.cs

@@ -297,8 +297,11 @@ public virtual string GetInitJavaScript()
 		}
 
 		/// <summary>
-		/// Attempts to execute the provided JavaScript code using the current engine. If an 
-		/// exception is thrown, retries the execution using a new thread (and hence a new engine)
+		/// Attempts to execute the provided JavaScript code using a non-pooled JavaScript engine (ie.
+		/// creates a new JS engine per-thread). This is because Babel uses a LOT of memory, so we 
+		/// should completely dispose any engines that have loaded Babel in order to conserve memory.
+		/// 
+		/// If an exception is thrown, retries the execution using a new thread (and hence a new engine)
 		/// with a larger maximum stack size.
 		/// This is required because JSXTransformer uses a huge stack which ends up being larger 
 		/// than what ASP.NET allows by default (256 KB).
@@ -307,18 +310,14 @@ public virtual string GetInitJavaScript()
 		/// <param name="function">JavaScript function to execute</param>
 		/// <param name="args">Arguments to pass to function</param>
 		/// <returns>Result returned from JavaScript code</returns>
-		public virtual T ExecuteWithLargerStackIfRequired<T>(string function, params object[] args)
+		public virtual T ExecuteWithBabel<T>(string function, params object[] args)
 		{
-			// This hack is not required when pooling JavaScript engines, since pooled MSIE
-			// engines already execute on their own thread with a larger stack.
-			if (_config.ReuseJavaScriptEngines)
-			{
-				return Execute<T>(function, args);
-			}
+			var engine = _engineFactory.GetEngineForCurrentThread();
+			EnsureBabelLoaded(engine);
 
 			try
 			{
-				return Execute<T>(function, args);
+				return engine.CallFunctionReturningJson<T>(function, args);
 			}
 			catch (Exception)
 			{
@@ -330,10 +329,11 @@ public virtual T ExecuteWithLargerStackIfRequired<T>(string function, params obj
 				var thread = new Thread(() =>
 				{
 					// New engine will be created here (as this is a new thread)
-					var engine = _engineFactory.GetEngineForCurrentThread();
+					var threadEngine = _engineFactory.GetEngineForCurrentThread();
+					EnsureBabelLoaded(threadEngine);
 					try
 					{
-						result = engine.CallFunctionReturningJson<T>(function, args);
+						result = threadEngine.CallFunctionReturningJson<T>(function, args);
 					}
 					catch (Exception threadEx)
 					{
@@ -417,18 +417,18 @@ public virtual IReactSiteConfiguration Configuration
 		/// <summary>
 		/// Ensures that Babel has been loaded into the JavaScript engine.
 		/// </summary>
-		public void EnsureBabelLoaded()
+		private void EnsureBabelLoaded(IJsEngine engine)
 		{
 			// If Babel is disabled in the config, don't even try loading it
 			if (!_config.LoadBabel)
 			{
 				throw new BabelNotLoadedException();
 			}
 
-			var babelLoaded = Engine.Evaluate<bool>("typeof global.Babel !== 'undefined'");
+			var babelLoaded = engine.Evaluate<bool>("typeof global.Babel !== 'undefined'");
 			if (!babelLoaded)
 			{
-				Engine.ExecuteResource("React.node_modules.babel_core.browser.js", typeof(ReactEnvironment).Assembly);
+				engine.ExecuteResource("React.node_modules.babel_core.browser.js", typeof(ReactEnvironment).Assembly);
 			}
 		}
 	}

src/React.Tests/Core/BabelTransformerTests.cs

@@ -49,7 +49,7 @@ public void ShouldTransformJsx()
 			const string input = "<div>Hello World</div>";
 			_babel.Transform(input);
 
-			_environment.Verify(x => x.ExecuteWithLargerStackIfRequired<string>(
+			_environment.Verify(x => x.ExecuteWithBabel<string>(
 				"ReactNET_transform",
 				"<div>Hello World</div>",
 				It.IsAny<string>(),
@@ -60,7 +60,7 @@ public void ShouldTransformJsx()
 		[Test]
 		public void ShouldWrapExceptionsInJsxExeption()
 		{
-			_environment.Setup(x => x.ExecuteWithLargerStackIfRequired<string>(
+			_environment.Setup(x => x.ExecuteWithBabel<string>(
 				"ReactNET_transform",
 				"<div>Hello World</div>",
 				It.IsAny<string>(),
@@ -103,7 +103,7 @@ public void ShouldTransformJsxIfFileCacheHashInvalid()
 			_fileSystem.Setup(x => x.ReadAsString("foo.generated.js")).Returns("/* filesystem cached invalid */");
 			_fileSystem.Setup(x => x.ReadAsString("foo.jsx")).Returns("<div>Hello World</div>");
 			_fileCacheHash.Setup(x => x.ValidateHash(It.IsAny<string>(), It.IsAny<string>())).Returns(false);
-			_environment.Setup(x => x.ExecuteWithLargerStackIfRequired<JavaScriptWithSourceMap>(
+			_environment.Setup(x => x.ExecuteWithBabel<JavaScriptWithSourceMap>(
 				"ReactNET_transform_sourcemap",
 				It.IsAny<string>(),
 				It.IsAny<string>(), // Babel config
@@ -120,7 +120,7 @@ public void ShouldTransformJsxIfNoCache()
 			SetUpEmptyCache();
 			_fileSystem.Setup(x => x.FileExists("foo.generated.js")).Returns(false);
 			_fileSystem.Setup(x => x.ReadAsString("foo.jsx")).Returns("<div>Hello World</div>");
-			_environment.Setup(x => x.ExecuteWithLargerStackIfRequired<JavaScriptWithSourceMap>(
+			_environment.Setup(x => x.ExecuteWithBabel<JavaScriptWithSourceMap>(
 				"ReactNET_transform_sourcemap",
 				It.IsAny<string>(),
 				It.IsAny<string>(), // Babel config
@@ -135,7 +135,7 @@ public void ShouldTransformJsxIfNoCache()
 		public void ShouldSaveTransformationResult()
 		{
 			_fileSystem.Setup(x => x.ReadAsString("foo.jsx")).Returns("<div>Hello World</div>");
-			_environment.Setup(x => x.ExecuteWithLargerStackIfRequired<JavaScriptWithSourceMap>(
+			_environment.Setup(x => x.ExecuteWithBabel<JavaScriptWithSourceMap>(
 				"ReactNET_transform_sourcemap",
 				It.IsAny<string>(),
 				It.IsAny<string>(), // Babel config

src/React.Tests/Core/ReactEnvironmentTest.cs

@@ -13,24 +13,25 @@
 using JavaScriptEngineSwitcher.Core;
 using Moq;
 using NUnit.Framework;
+using React.Exceptions;
 
 namespace React.Tests.Core
 {
 	[TestFixture]
 	public class ReactEnvironmentTest
 	{
 		[Test]
-		public void ExecuteWithLargerStackIfRequiredWithNoNewThread()
+		public void ExecuteWithBabelWithNoNewThread()
 		{
 			var mocks = new Mocks();
 			var environment = mocks.CreateReactEnvironment();
 
-			environment.ExecuteWithLargerStackIfRequired<int>("foo");
+			environment.ExecuteWithBabel<int>("foo");
 			mocks.Engine.Verify(x => x.CallFunction<int>("foo"), Times.Exactly(1));
 		}
 
 		[Test]
-		public void ExecuteWithLargerStackIfRequiredWithNewThread()
+		public void ExecuteWithBabelWithNewThread()
 		{
 			var mocks = new Mocks();
 			var environment = mocks.CreateReactEnvironment();
@@ -40,7 +41,7 @@ public void ExecuteWithLargerStackIfRequiredWithNewThread()
 				.Callback(() => mocks.Engine.Setup(x => x.CallFunction<int>("foo")))
 				.Throws(new Exception("Out of stack space"));
 
-			environment.ExecuteWithLargerStackIfRequired<int>("foo");
+			environment.ExecuteWithBabel<int>("foo");
 			mocks.EngineFactory.Verify(
 				x => x.GetEngineForCurrentThread(), 
 				Times.Exactly(2),
@@ -54,7 +55,7 @@ public void ExecuteWithLargerStackIfRequiredWithNewThread()
 		}
 
 		[Test]
-		public void ExecuteWithLargerStackIfRequiredShouldBubbleExceptions()
+		public void ExecuteWithBabelShouldBubbleExceptions()
 		{
 			var mocks = new Mocks();
 			var environment = mocks.CreateReactEnvironment();
@@ -64,7 +65,20 @@ public void ExecuteWithLargerStackIfRequiredShouldBubbleExceptions()
 
 			Assert.Throws<Exception>(() =>
 			{
-				environment.ExecuteWithLargerStackIfRequired<int>("foobar");
+				environment.ExecuteWithBabel<int>("foobar");
+			});
+		}
+
+		[Test]
+		public void ExecuteWithBabelShouldThrowIfBabelDisabled()
+		{
+			var mocks = new Mocks();
+			mocks.Config.Setup(x => x.LoadBabel).Returns(false);
+			var environment = mocks.CreateReactEnvironment();
+
+			Assert.Throws<BabelNotLoadedException>(() =>
+			{
+				environment.ExecuteWithBabel<string>("foobar");
 			});
 		}
 
@@ -112,6 +126,7 @@ public Mocks()
 				FileCacheHash = new Mock<IFileCacheHash>();
 
 				EngineFactory.Setup(x => x.GetEngineForCurrentThread()).Returns(Engine.Object);
+				Config.Setup(x => x.LoadBabel).Returns(true);
 			}
 
 			public ReactEnvironment CreateReactEnvironment()