diff --git a/src/Framework/Framework/Compilation/ErrorCheckingVisitor.cs b/src/Framework/Framework/Compilation/ErrorCheckingVisitor.cs index 5f6a63752..86413cde7 100644 --- a/src/Framework/Framework/Compilation/ErrorCheckingVisitor.cs +++ b/src/Framework/Framework/Compilation/ErrorCheckingVisitor.cs @@ -71,7 +71,7 @@ private void AddNodeErrors(DothtmlNode node, int priority) /// * the locations are processed using MapBindingLocation to make them useful in the context of a dothtml file Dictionary AnnotateBindingExceptionWithLocation(ResolvedBinding binding, DotvvmProperty? relatedProperty, IEnumerable errors) { - var result = new Dictionary(new ReferenceEqualityComparer()); + var result = new Dictionary(ReferenceEqualityComparer.Instance); void recurse(Exception exception, DotvvmCompilationSourceLocation? location) { if (result.ContainsKey(exception)) diff --git a/src/Framework/Framework/Controls/Repeater.cs b/src/Framework/Framework/Controls/Repeater.cs index 5b7095924..988f8bbf2 100644 --- a/src/Framework/Framework/Controls/Repeater.cs +++ b/src/Framework/Framework/Controls/Repeater.cs @@ -10,6 +10,7 @@ using System.Linq; using System.Runtime.CompilerServices; using System.Collections.Generic; +using DotVVM.Framework.Utils; namespace DotVVM.Framework.Controls { @@ -139,6 +140,7 @@ protected internal override void OnLoad(IDotvvmRequestContext context) protected internal override void OnPreRender(IDotvvmRequestContext context) { SetChildren(context, renderClientTemplate: !RenderOnServer, memoizeReferences: false); + childrenCache.Clear(); // not going to need the unreferenced children anymore base.OnPreRender(context); } @@ -248,7 +250,7 @@ private DotvvmControl CreateEmptyItem(IDotvvmRequestContext context) return emptyDataContainer; } - private ConditionalWeakTable childrenCache = new ConditionalWeakTable(); + private readonly Dictionary childrenCache = new(ReferenceEqualityComparer.Instance); private DotvvmControl AddItem(IList c, IDotvvmRequestContext context, object? item = null, int? index = null, bool allowMemoizationRetrieve = false, bool allowMemoizationStore = false) { if (allowMemoizationRetrieve && item != null && childrenCache.TryGetValue(item, out var container2) && container2.Parent == null) @@ -276,8 +278,12 @@ private DotvvmControl AddItem(IList c, IDotvvmRequestContext cont // write it to the cache after the content is build. If it would be before that, exception could be suppressed if (allowMemoizationStore && item != null) { - // this GetValue call adds the value without raising exception when the value is already added... - childrenCache.GetValue(item, _ => container); +#if DotNetCore + childrenCache.TryAdd(item, container); +#else + if (!childrenCache.ContainsKey(item)) + childrenCache.Add(item, container); +#endif } return container; @@ -306,6 +312,7 @@ private void SetChildren(IDotvvmRequestContext context, bool renderClientTemplat if (DataSource != null) { var index = 0; + var isCommand = context.RequestType == DotvvmRequestType.Command; // on GET request we are not initializing the Repeater twice foreach (var item in GetIEnumerableFromDataSource()!) { if (SeparatorTemplate != null && index > 0) @@ -313,7 +320,7 @@ private void SetChildren(IDotvvmRequestContext context, bool renderClientTemplat AddSeparator(Children, context); } AddItem(Children, context, item, index, - allowMemoizationRetrieve: context.RequestType == DotvvmRequestType.Command && !memoizeReferences, // on GET request we are not initializing the Repeater twice + allowMemoizationRetrieve: isCommand && !memoizeReferences, allowMemoizationStore: memoizeReferences ); index++; diff --git a/src/Framework/Framework/Utils/ReferenceEqualityComparer.cs b/src/Framework/Framework/Utils/ReferenceEqualityComparer.cs index 8d83b2f6f..4c34d4126 100644 --- a/src/Framework/Framework/Utils/ReferenceEqualityComparer.cs +++ b/src/Framework/Framework/Utils/ReferenceEqualityComparer.cs @@ -1,12 +1,16 @@ using System.Collections.Generic; +using System.Runtime.CompilerServices; namespace DotVVM.Framework.Utils { + // TODO next version: Replace with System.Collections.Generic.ReferenceEqualityComparer internal class ReferenceEqualityComparer : IEqualityComparer where T : class { - public bool Equals(T? x, T? y) => ReferenceEquals(x, y); + public static ReferenceEqualityComparer Instance { get; } = new ReferenceEqualityComparer(); + - public int GetHashCode(T obj) => obj?.GetHashCode() ?? 0; + public bool Equals(T? x, T? y) => ReferenceEquals(x, y); + public int GetHashCode(T obj) => obj == null ? 0 : RuntimeHelpers.GetHashCode(obj); } } diff --git a/src/Tests/Runtime/RepeaterMemoizationTest.cs b/src/Tests/Runtime/RepeaterMemoizationTest.cs new file mode 100644 index 000000000..9c447993c --- /dev/null +++ b/src/Tests/Runtime/RepeaterMemoizationTest.cs @@ -0,0 +1,157 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using System.Linq; +using AngleSharp.Io; +using DotVVM.Framework.Binding.Expressions; +using DotVVM.Framework.Compilation.ControlTree; +using DotVVM.Framework.Controls; +using DotVVM.Framework.Controls.Infrastructure; +using DotVVM.Framework.Hosting; +using DotVVM.Framework.Testing; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace DotVVM.Framework.Tests.Runtime +{ + [TestClass] + public class RepeaterMemoizationTest: DotvvmControlTestBase + { + (Repeater, TestDotvvmRequestContext) Init(T viewModel, ITemplate template) + where T: IEnumerable + { + var dataContextStack = DataContextStack.Create(viewModel.GetType()); + var repeater = new Repeater() { + DataSource = ValueBindingExpression.CreateBinding(BindingService, h => (T)h[0], dataContextStack), + ItemTemplate = template + }; + repeater.SetValue(RenderSettings.ModeProperty, RenderMode.Client); + + var context = CreateContext(viewModel); + + return (repeater, context); + } + + [DataTestMethod] + [DataRow(DotvvmRequestType.Navigate, RenderMode.Client)] + [DataRow(DotvvmRequestType.Navigate, RenderMode.Server)] + [DataRow(DotvvmRequestType.SpaNavigate, RenderMode.Client)] + [DataRow(DotvvmRequestType.SpaNavigate, RenderMode.Server)] + [DataRow(DotvvmRequestType.Command, RenderMode.Client)] + [DataRow(DotvvmRequestType.Command, RenderMode.Server)] + public void Repeater_TemplateInitializedOnce(DotvvmRequestType requestType, RenderMode renderMode) + { + var templateInits = new List(); + var (repeater, context) = Init(new[] { "ROW 1", "ROW 2", "ROW 3" }, new DelegateTemplate((sp, body) => { + + body.AppendChildren(new Literal("test")); + templateInits.Add((string)body.DataContext); + })); + repeater.SetValue(RenderSettings.ModeProperty, renderMode); + context.RequestType = requestType; + var html = InvokeLifecycleAndRender(repeater, context); + + Console.WriteLine("Template inits: " + string.Join(", ", templateInits.Select(i => i ?? ""))); + + if (renderMode == RenderMode.Client) + { + XAssert.Equal(["ROW 1", "ROW 2", "ROW 3", null], templateInits); + } + else + { + XAssert.Equal(["ROW 1", "ROW 2", "ROW 3"], templateInits); + } + } + + [DataTestMethod] + [DataRow(DotvvmRequestType.Navigate, RenderMode.Client)] + [DataRow(DotvvmRequestType.Navigate, RenderMode.Server)] + [DataRow(DotvvmRequestType.SpaNavigate, RenderMode.Client)] + [DataRow(DotvvmRequestType.SpaNavigate, RenderMode.Server)] + [DataRow(DotvvmRequestType.Command, RenderMode.Client)] + [DataRow(DotvvmRequestType.Command, RenderMode.Server)] + public void Repeater_ViewModelChange(DotvvmRequestType requestType, RenderMode renderMode) + { + var templateInits = new List(); + var viewModel = new[] { "ROW 1", "ROW 2", "ROW 3" }; + var (repeater, context) = Init(viewModel, new DelegateTemplate((sp, body) => { + + body.AppendChildren(new Literal("test")); + templateInits.Add((string)body.DataContext); + })); + context.RequestType = requestType; + repeater.SetValue(RenderSettings.ModeProperty, renderMode); + + var view = new DotvvmView(); + view.Children.Add(repeater); + view.SetValue(Internal.RequestContextProperty, context); + view.DataContext = context.ViewModel; + + // Run Load event + + DotvvmControlCollection.InvokePageLifeCycleEventRecursive(view, LifeCycleEventType.Load, context); + Console.WriteLine("Template inits (Load): " + string.Join(", ", templateInits.Select(i => i ?? ""))); + + if (requestType == DotvvmRequestType.Command) + XAssert.Equal(["ROW 1", "ROW 2", "ROW 3"], templateInits); + else + XAssert.Equal([], templateInits); + + // Continue with PreRender and Render after shuffling the view model + + view.DataContext = context.ViewModel = viewModel = [ viewModel[1], viewModel[2], "ROW 4", viewModel[1] ]; + + var html = InvokeLifecycleAndRender(view, context); + + Console.WriteLine("Template inits (Render): " + string.Join(", ", templateInits.Select(i => i ?? ""))); + + string[] nullIfClient = renderMode == RenderMode.Client ? [ null ] : []; + + if (requestType == DotvvmRequestType.Command) + { + XAssert.Equal(["ROW 1", "ROW 2", "ROW 3", /* Load end */ "ROW 4", "ROW 2", ..nullIfClient], templateInits); + } + else + { + XAssert.Equal(["ROW 2", "ROW 3", "ROW 4", "ROW 2", ..nullIfClient], templateInits); + } + } + + [DataTestMethod] + [DataRow(DotvvmRequestType.Navigate, RenderMode.Client)] + [DataRow(DotvvmRequestType.Navigate, RenderMode.Server)] + [DataRow(DotvvmRequestType.SpaNavigate, RenderMode.Client)] + [DataRow(DotvvmRequestType.SpaNavigate, RenderMode.Server)] + [DataRow(DotvvmRequestType.Command, RenderMode.Client)] + [DataRow(DotvvmRequestType.Command, RenderMode.Server)] + public void Repeater_IsCollectible(DotvvmRequestType requestType, RenderMode renderMode) + { + var (repeaterRef, contextRef, viewModelRef) = RunLifecycle(requestType, renderMode); + + for (int i = 0; i < 100; i++) // single collection is enough normally, but just to be sure it's not flaky on CI... + { + GC.Collect(2, GCCollectionMode.Forced, blocking: true); + + Console.WriteLine($"GC Collect #{i} ({repeaterRef.IsAlive}, {contextRef.IsAlive}, {viewModelRef.IsAlive})"); + + if (!repeaterRef.IsAlive || !contextRef.IsAlive || !viewModelRef.IsAlive) + { + break; + } + } + + XAssert.Equal((false, false, false), (repeaterRef.IsAlive, contextRef.IsAlive, viewModelRef.IsAlive)); + + (WeakReference, WeakReference, WeakReference) RunLifecycle(DotvvmRequestType requestType, RenderMode renderMode) + { + var (repeater, context) = Init(new[] { "ROW 1", "ROW 2", "ROW 3" }, new DelegateTemplate((sp, body) => { + body.AppendChildren(new Literal("test")); + })); + repeater.SetValue(RenderSettings.ModeProperty, renderMode); + context.RequestType = requestType; + var html = InvokeLifecycleAndRender(repeater, context); + + return (new WeakReference(repeater), new WeakReference(context), new WeakReference(context.ViewModel)); + } + } + } +}