Skip to content

Commit

Permalink
Fix excessive allocations in EmbeddedResourceFileLoader
Browse files Browse the repository at this point in the history
Previously we'd load the entire resource on every invocation of GetControlBuilder,
which might be a number of times per request in case of markup controls.
  • Loading branch information
exyi committed Nov 23, 2024
1 parent 1b8d606 commit e67a069
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 7 deletions.
19 changes: 12 additions & 7 deletions src/Framework/Framework/Hosting/EmbeddedMarkupFileLoader.cs
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
using System;
using System.Buffers;
using System.Collections.Generic;
using System.IO;
using System.Reflection;
using System.Text;
using DotVVM.Framework.Configuration;
using DotVVM.Framework.Utils;

namespace DotVVM.Framework.Hosting
{
/// <summary> Read markup files from embedded resources, if the virtual path has the following format: <c>embedded://{AssemblyName}/{ResourceName}</c></summary>
public class EmbeddedMarkupFileLoader : IMarkupFileLoader
{
/// <summary>
Expand All @@ -23,7 +26,7 @@ public class EmbeddedMarkupFileLoader : IMarkupFileLoader

if (resourceName.IndexOf('/') == -1 || resourceName.IndexOf('/') == 0)
{
throw new ArgumentException("Wrong string format", "virtualPath");
throw new ArgumentException("Wrong embedded file format. Use `embedded://{AssemblyName}/{ResourceName}`", "virtualPath");
}

string assemblyName = resourceName.Substring(0, resourceName.IndexOf('/'));
Expand All @@ -37,20 +40,22 @@ public class EmbeddedMarkupFileLoader : IMarkupFileLoader
//no such assembly found
catch (FileLoadException)
{
throw new ArgumentException("No such assembly was found", "virtualPath");
throw new ArgumentException($"Assembly '{assemblyName}' was not found", "virtualPath");
}

//no such resource found
resourceName = resourceName.Replace('/', '.');
if (assembly.GetManifestResourceInfo(resourceName) == null)
{
throw new ArgumentException("No such resource was found", "virtualPath");
throw new ArgumentException($"Resource '{resourceName}' was not found in assembly '{assembly.FullName}'", "virtualPath");
}

//load the file
using (Stream stream = assembly.GetManifestResourceStream(resourceName)!)
using (StreamReader sr = new StreamReader(stream))
return new MarkupFile(virtualPath, virtualPath, sr.ReadToEnd());
return new MarkupFile(virtualPath, virtualPath, () => {
//load the file
using (Stream stream = assembly.GetManifestResourceStream(resourceName)!)
using (var reader = new StreamReader(stream))
return reader.ReadToEnd();
});
}

/// <summary>
Expand Down
8 changes: 8 additions & 0 deletions src/Framework/Framework/Hosting/MarkupFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ public MarkupFile(string fileName, string fullPath)
};
}

public MarkupFile(string fileName, string fullPath, Func<string> readContent, DateTime lastWriteDateTimeUtc = default)
{
FileName = fileName;
FullPath = fullPath;
ReadContent = readContent;
LastWriteDateTimeUtc = lastWriteDateTimeUtc;
}

public MarkupFile(string fileName, string fullPath, string contents, DateTime lastWriteDateTimeUtc = default)
{
FileName = fileName;
Expand Down
12 changes: 12 additions & 0 deletions src/Framework/Testing/DotvvmTestHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,23 @@ public static void RegisterMockServices(IServiceCollection services)
var config = CreateConfiguration();
config.ExperimentalFeatures.UseDotvvmSerializationForStaticCommandArguments.Enable();
config.RouteTable.Add("TestRoute", "TestRoute", "TestView.dothtml");
config.Diagnostics.Apply(config);
config.Freeze();
return config;
});
public static DotvvmConfiguration DefaultConfig => _defaultConfig.Value;

private static Lazy<DotvvmConfiguration> _debugConfig = new Lazy<DotvvmConfiguration>(() => {
var config = CreateConfiguration();
config.ExperimentalFeatures.UseDotvvmSerializationForStaticCommandArguments.Enable();
config.RouteTable.Add("TestRoute", "TestRoute", "TestView.dothtml");
config.Debug = true;
config.Diagnostics.Apply(config);
config.Freeze();
return config;
});
public static DotvvmConfiguration DebugConfig => _debugConfig.Value;

public static DotvvmConfiguration CreateConfiguration(Action<IServiceCollection>? customServices = null) =>
DotvvmConfiguration.CreateDefault(s => {
s.AddSingleton<ITestSingletonService, TestSingletonService>();
Expand Down
112 changes: 112 additions & 0 deletions src/Tests/Runtime/MarkupLoaderTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using DotVVM.Framework.Compilation;
using DotVVM.Framework.Configuration;
using DotVVM.Framework.Controls;
using DotVVM.Framework.Hosting;
using DotVVM.Framework.Runtime;
using DotVVM.Framework.Testing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace DotVVM.Framework.Tests.Runtime
{
[TestClass]
public class MarkupLoaderTests: IDisposable
{
readonly string compilationPageResource = DotvvmTestHelper.DebugConfig.RouteTable[DotvvmCompilationPageConfiguration.DefaultRouteName].VirtualPath;

readonly List<string> tempFiles = [];

[TestMethod]
public void EmbeddedResource()
{
var loader = new EmbeddedMarkupFileLoader();
var file = loader.GetMarkup(DotvvmTestHelper.DebugConfig, compilationPageResource);

XAssert.StartsWith("@viewModel DotVVM.Framework.Diagnostics.CompilationPageViewModel", file.ReadContent());
}

#if DotNetCore
[TestMethod]
public void EmbeddedResourceLazyAllocation()
{
var loader = new EmbeddedMarkupFileLoader();
// warmup for assembly loading and such
loader.GetMarkup(DotvvmTestHelper.DebugConfig, compilationPageResource);

// GetMarkup allocates constant memory, as it is being called repeatedly if file reloading is enabled
var a = GC.GetAllocatedBytesForCurrentThread();
var file = loader.GetMarkup(DotvvmTestHelper.DebugConfig, compilationPageResource);
var b = GC.GetAllocatedBytesForCurrentThread();
XAssert.InRange(b - a, 0, 1000);

// ReadContent actually reads the file and allocates the string
a = GC.GetAllocatedBytesForCurrentThread();
var content = file.ReadContent();
b = GC.GetAllocatedBytesForCurrentThread();
XAssert.InRange(content.Length, 1000, int.MaxValue);
XAssert.InRange(b - a, content.Length * 2, content.Length * 5);
}
#endif

[TestMethod]
[DataRow(true)]
[DataRow(false)]
public void FileReloading(bool debug)
{
var directory = MakeTempDir();
var file = Path.Combine(directory, "test.dotcontrol");
File.WriteAllText(file, "@viewModel string\n\n<dot:TextBox Text=Initial />");

var config = debug ? DotvvmTestHelper.DebugConfig : DotvvmTestHelper.DefaultConfig;

var controlBuilder = config.ServiceProvider.GetRequiredService<IControlBuilderFactory>();
var builder0 = controlBuilder.GetControlBuilder(file);
Assert.AreEqual(typeof(string), builder0.descriptor.DataContextType);

var builderUnchanged = controlBuilder.GetControlBuilder(file);

Assert.AreSame(builder0.builder, builderUnchanged.builder); // same Lazy instance

File.WriteAllText(file, "@viewModel int\n\n<dot:TextBox Text=Changed />");

var builderChanged = controlBuilder.GetControlBuilder(file);
var control = builderChanged.builder.Value.BuildControl(config.ServiceProvider.GetRequiredService<IControlBuilderFactory>(), config.ServiceProvider);
if (debug)
{
Assert.AreEqual(typeof(int), builderChanged.descriptor.DataContextType);

Check failure on line 80 in src/Tests/Runtime/MarkupLoaderTests.cs

View workflow job for this annotation

GitHub Actions / .NET unit tests (windows-2022)

FileReloading (True)

Assert.AreEqual failed. Expected:<System.Int32>. Actual:<System.String>.

Check failure on line 80 in src/Tests/Runtime/MarkupLoaderTests.cs

View workflow job for this annotation

GitHub Actions / .NET unit tests (windows-2022)

FileReloading (True)

Assert.AreEqual failed. Expected:<System.Int32>. Actual:<System.String>.
Assert.AreNotSame(builder0.builder, builderChanged.builder); // different Lazy instance
XAssert.Equal(["Changed"], control.GetThisAndAllDescendants().OfType<TextBox>().Select(c => c.Text));
}
else
{
// not reloaded in Release mode by default
Assert.AreEqual(typeof(string), builderChanged.descriptor.DataContextType);
Assert.AreSame(builder0.builder, builderChanged.builder); // different Lazy instance
XAssert.Equal(["Initial"], control.GetThisAndAllDescendants().OfType<TextBox>().Select(c => c.Text));
}
}

public string MakeTempDir()
{
var path = Path.Combine(Path.GetTempPath(), "dotvvm-tests-tmp-" + Path.GetRandomFileName());
Directory.CreateDirectory(path);
tempFiles.Add(path);
return path;
}

public void Dispose()
{
foreach (var file in tempFiles)
{
if (Directory.Exists(file))
Directory.Delete(file, true);
else if (File.Exists(file))
File.Delete(file);
}
}
}
}

0 comments on commit e67a069

Please sign in to comment.