Skip to content

Always return JS engine to pool after component render. #270

New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 22, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 49 additions & 28 deletions src/React.AspNet/HtmlHelperExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,17 +81,24 @@ public static IHtmlString React<T>(
string containerClass = null
)
{
var reactComponent = Environment.CreateComponent(componentName, props, containerId, clientOnly);
if (!string.IsNullOrEmpty(htmlTag))
try
{
reactComponent.ContainerTag = htmlTag;
var reactComponent = Environment.CreateComponent(componentName, props, containerId, clientOnly);
if (!string.IsNullOrEmpty(htmlTag))
{
reactComponent.ContainerTag = htmlTag;
}
if (!string.IsNullOrEmpty(containerClass))
{
reactComponent.ContainerClass = containerClass;
}
var result = reactComponent.RenderHtml(clientOnly, serverOnly);
return new HtmlString(result);
}
if (!string.IsNullOrEmpty(containerClass))
finally
{
reactComponent.ContainerClass = containerClass;
Environment.ReturnEngineToPool();
}
var result = reactComponent.RenderHtml(clientOnly, serverOnly);
return new HtmlString(result);
}

/// <summary>
Expand All @@ -114,31 +121,38 @@ public static IHtmlString ReactWithInit<T>(
T props,
string htmlTag = null,
string containerId = null,
bool clientOnly = false,
bool clientOnly = false,
string containerClass = null
)
{
var reactComponent = Environment.CreateComponent(componentName, props, containerId, clientOnly);
if (!string.IsNullOrEmpty(htmlTag))
{
reactComponent.ContainerTag = htmlTag;
}
if (!string.IsNullOrEmpty(containerClass))
try
{
reactComponent.ContainerClass = containerClass;
}
var html = reactComponent.RenderHtml(clientOnly);
var reactComponent = Environment.CreateComponent(componentName, props, containerId, clientOnly);
if (!string.IsNullOrEmpty(htmlTag))
{
reactComponent.ContainerTag = htmlTag;
}
if (!string.IsNullOrEmpty(containerClass))
{
reactComponent.ContainerClass = containerClass;
}
var html = reactComponent.RenderHtml(clientOnly);

#if LEGACYASPNET
var script = new TagBuilder("script")
{
InnerHtml = reactComponent.RenderJavaScript()
};
var script = new TagBuilder("script")
{
InnerHtml = reactComponent.RenderJavaScript()
};
#else
var script = new TagBuilder("script");
script.InnerHtml.AppendHtml(reactComponent.RenderJavaScript());
#endif
return new HtmlString(html + System.Environment.NewLine + script.ToString());
return new HtmlString(html + System.Environment.NewLine + script.ToString());
}
finally
{
Environment.ReturnEngineToPool();
}
}

/// <summary>
Expand All @@ -148,18 +162,25 @@ public static IHtmlString ReactWithInit<T>(
/// <returns>JavaScript for all components</returns>
public static IHtmlString ReactInitJavaScript(this IHtmlHelper htmlHelper, bool clientOnly = false)
{
var script = Environment.GetInitJavaScript(clientOnly);
#if LEGACYASPNET
var tag = new TagBuilder("script")
try
{
InnerHtml = script
};
return new HtmlString(tag.ToString());
var script = Environment.GetInitJavaScript(clientOnly);
#if LEGACYASPNET
var tag = new TagBuilder("script")
{
InnerHtml = script
};
return new HtmlString(tag.ToString());
#else
var tag = new TagBuilder("script");
tag.InnerHtml.AppendHtml(script);
return tag;
#endif
}
finally
{
Environment.ReturnEngineToPool();
}
}
}
}
5 changes: 5 additions & 0 deletions src/React.Core/IReactEnvironment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,5 +96,10 @@ public interface IReactEnvironment
/// Gets the JSX Transformer for this environment.
/// </summary>
IBabel Babel { get; }

/// <summary>
/// Returns the currently held JS engine to the pool. (no-op if engine pooling is disabled)
/// </summary>
void ReturnEngineToPool();
}
}
11 changes: 10 additions & 1 deletion src/React.Core/ReactEnvironment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public class ReactEnvironment : IReactEnvironment, IDisposable
/// Contains an engine acquired from a pool of engines. Only used if
/// <see cref="IReactSiteConfiguration.ReuseJavaScriptEngines"/> is enabled.
/// </summary>
protected readonly Lazy<IJsEngine> _engineFromPool;
protected Lazy<IJsEngine> _engineFromPool;

/// <summary>
/// List of all components instantiated in this environment
Expand Down Expand Up @@ -379,9 +379,18 @@ private static string GetVersion()
public virtual void Dispose()
{
_engineFactory.DisposeEngineForCurrentThread();
ReturnEngineToPool();
}

/// <summary>
/// Returns the currently held JS engine to the pool. (no-op if engine pooling is disabled)
/// </summary>
public void ReturnEngineToPool()
{
if (_engineFromPool.IsValueCreated)
{
_engineFactory.ReturnEngineToPool(_engineFromPool.Value);
_engineFromPool = new Lazy<IJsEngine>(() => _engineFactory.GetEngine());
}
}

Expand Down
16 changes: 16 additions & 0 deletions src/React.Tests/Core/ReactEnvironmentTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,21 @@ public void UsesProvidedContainerId()
StringAssert.StartsWith("react_", component2.ContainerId);
}

[Test]
public void ReturnsEngineToPool()
{
var mocks = new Mocks();
var environment = mocks.CreateReactEnvironment();
mocks.Config.Setup(x => x.ReuseJavaScriptEngines).Returns(true);

environment.CreateComponent("ComponentName", new { });
mocks.EngineFactory.Verify(x => x.GetEngine(), Times.Once);
environment.ReturnEngineToPool();

environment.CreateComponent("ComponentName", new { });
mocks.EngineFactory.Verify(x => x.GetEngine(), Times.AtLeast(2));
}

private class Mocks
{
public Mock<IJsEngine> Engine { get; private set; }
Expand All @@ -125,6 +140,7 @@ public Mocks()
FileSystem = new Mock<IFileSystem>();
FileCacheHash = new Mock<IFileCacheHash>();

EngineFactory.Setup(x => x.GetEngine()).Returns(Engine.Object);
EngineFactory.Setup(x => x.GetEngineForCurrentThread()).Returns(Engine.Object);
Config.Setup(x => x.LoadBabel).Returns(true);
}
Expand Down