Merge pull request #1888 from riganti/fix-embeddedresource-markup-all…

…ocation

Fix excessive allocations in EmbeddedResourceFileLoader

src/Framework/Framework/Hosting/EmbeddedMarkupFileLoader.cs

@@ -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>
@@ -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('/'));
@@ -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>

src/Framework/Framework/Hosting/MarkupFile.cs

@@ -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;

src/Framework/Testing/DotvvmTestHelper.cs

@@ -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>();

src/Tests/Runtime/MarkupLoaderTests.cs

@@ -0,0 +1,118 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Threading;
+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 changedTime1 = File.GetLastWriteTimeUtc(file);
+
+            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)
+            {
+                var changedTime2 = File.GetLastWriteTimeUtc(file);
+                if (changedTime1 == changedTime2)
+                    Assert.Inconclusive($"File system resolution is probably too low ({changedTime1:o} == {changedTime2:o}), ignores changes or something.");
+
+                Assert.AreEqual(typeof(int), builderChanged.descriptor.DataContextType);
+                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);
+            }
+        }
+    }
+}