From 25b76bc6dcc063edd58434a4af6c42e336c2f6fc Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 22 Feb 2018 13:23:52 +0000 Subject: [PATCH] Skip rerendering child components if their params are definitely unchanged --- .../Components/ParameterCollection.cs | 95 +++++++++++++++++++ .../RenderTree/RenderTreeDiffBuilder.cs | 17 +++- .../RenderTreeDiffBuilderTest.cs | 52 ++++++++++ .../RendererTest.cs | 2 +- 4 files changed, 161 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.AspNetCore.Blazor/Components/ParameterCollection.cs b/src/Microsoft.AspNetCore.Blazor/Components/ParameterCollection.cs index 1c8df0dbe4..0a0c83cb83 100644 --- a/src/Microsoft.AspNetCore.Blazor/Components/ParameterCollection.cs +++ b/src/Microsoft.AspNetCore.Blazor/Components/ParameterCollection.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using Microsoft.AspNetCore.Blazor.RenderTree; namespace Microsoft.AspNetCore.Blazor.Components @@ -39,5 +40,99 @@ namespace Microsoft.AspNetCore.Blazor.Components /// The enumerator. public ParameterEnumerator GetEnumerator() => new ParameterEnumerator(_frames, _ownerIndex); + + // It's internal because there isn't a known use case for user code comparing + // ParameterCollection instances, and even if there was, it's unlikely it should + // use these equality rules which are designed for their effect on rendering. + internal bool DefinitelyEquals(ParameterCollection oldParameters) + { + // In general we can't detect mutations on arbitrary objects. We can't trust + // things like .Equals or .GetHashCode because they usually only tell us about + // shallow changes, not deep mutations. So we return false if both: + // [1] All the parameters are known to be immutable (i.e., Type.IsPrimitive + // or is in a known set of common immutable types) + // [2] And all the parameter values are equal to their previous values + // Otherwise be conservative and return false. + // To make this check cheaper, since parameters are virtually always generated in + // a deterministic order, we don't bother to account for reordering, so if any + // of the names don't match sequentially we just return false too. + // + // The logic here may look kind of epic, and would certainly be simpler if we + // used ParameterEnumerator.GetEnumerator(), but it's perf-critical and this + // implementation requires a lot fewer instructions than a GetEnumerator-based one. + + var oldIndex = oldParameters._ownerIndex; + var newIndex = _ownerIndex; + var oldEndIndexExcl = oldIndex + oldParameters._frames[oldIndex].ComponentSubtreeLength; + var newEndIndexExcl = newIndex + _frames[newIndex].ComponentSubtreeLength; + while (true) + { + // First, stop if we've reached the end of either subtree + oldIndex++; + newIndex++; + var oldFinished = oldIndex == oldEndIndexExcl; + var newFinished = newIndex == newEndIndexExcl; + if (oldFinished || newFinished) + { + return oldFinished == newFinished; // Same only if we have same number of parameters + } + else + { + // Since neither subtree has finished, it's safe to read the next frame from both + ref var oldFrame = ref oldParameters._frames[oldIndex]; + ref var newFrame = ref _frames[newIndex]; + + // Stop if we've reached the end of either subtree's sequence of attributes + oldFinished = oldFrame.FrameType != RenderTreeFrameType.Attribute; + newFinished = newFrame.FrameType != RenderTreeFrameType.Attribute; + if (oldFinished || newFinished) + { + return oldFinished == newFinished; // Same only if we have same number of parameters + } + else + { + if (!string.Equals(oldFrame.AttributeName, newFrame.AttributeName, StringComparison.Ordinal)) + { + return false; // Different names + } + + var oldValue = oldFrame.AttributeValue; + var newValue = newFrame.AttributeValue; + var oldIsNotNull = oldValue != null; + var newIsNotNull = newValue != null; + if (oldIsNotNull != newIsNotNull) + { + return false; // One's null and the other isn't, so different + } + else if (oldIsNotNull) // i.e., both are not null (considering previous check) + { + var oldValueType = oldValue.GetType(); + var newValueType = newValue.GetType(); + if (oldValueType != newValueType // Definitely different + || !IsKnownImmutableType(oldValueType) // Maybe different + || !oldValue.Equals(newValue)) // Somebody says they are different + { + return false; + } + } + else + { + // Both null, hence equal, so continue + } + } + } + } + } + + // The contents of this list need to trade off false negatives against computation + // time. So we don't want a huge list of types to check (or would have to move to + // a hashtable lookup, which is differently expensive). It's better not to include + // uncommon types here even if they are known to be immutable. + private bool IsKnownImmutableType(Type type) + => type.IsPrimitive + || type == typeof(string) + || type == typeof(DateTime) + || type == typeof(RenderFragment) + || type == typeof(decimal); } } diff --git a/src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeDiffBuilder.cs b/src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeDiffBuilder.cs index 90ea174566..12952e2945 100644 --- a/src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeDiffBuilder.cs +++ b/src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeDiffBuilder.cs @@ -153,13 +153,22 @@ namespace Microsoft.AspNetCore.Blazor.RenderTree // Preserve the actual componentInstance newComponentFrame = newComponentFrame.WithComponentInstance(componentId, componentInstance); - // Supply latest parameters. They might not have changed, but it's up to the - // recipient to decide what "changed" means. Currently we only supply the new + // As an important rendering optimization, we want to skip parameter update + // notifications if we know for sure they haven't changed/mutated. The + // "MayHaveChangedSince" logic is conservative, in that it returns true if + // any parameter is of a type we don't know is immutable. In this case + // we call SetParameters and it's up to the recipient to implement + // whatever change-detection logic they want. Currently we only supply the new // set of parameters and assume the recipient has enough info to do whatever // comparisons it wants with the old values. Later we could choose to pass the - // old parameter values if we wanted. + // old parameter values if we wanted. By default, components always rerender + // after any SetParameters call, which is safe but now always optimal for perf. + var oldParameters = new ParameterCollection(oldTree, oldComponentIndex); var newParameters = new ParameterCollection(newTree, newComponentIndex); - componentInstance.SetParameters(newParameters); + if (!newParameters.DefinitelyEquals(oldParameters)) + { + componentInstance.SetParameters(newParameters); + } } private static int NextSiblingIndex(RenderTreeFrame frame, int frameIndex) diff --git a/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffBuilderTest.cs b/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffBuilderTest.cs index f94ee50ac4..bb215cb513 100644 --- a/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffBuilderTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffBuilderTest.cs @@ -832,6 +832,44 @@ namespace Microsoft.AspNetCore.Blazor.Test Assert.Same(objectWillNotChange, newComponentInstance.ObjectProperty); } + [Fact] + public void SkipsUpdatingParametersOnChildComponentsIfAllAreDefinitelyImmutableAndUnchanged() + { + // We only know that types are immutable if either Type.IsPrimitive, or it's one of + // a known set of common immutable types. + + // Arrange: Populate old and new with equivalent content + RenderFragment fragmentWillNotChange = builder => throw new NotImplementedException(); + var dateTimeWillNotChange = DateTime.Now; + foreach (var tree in new[] { oldTree, newTree }) + { + tree.OpenComponent(0); + tree.AddAttribute(1, "MyString", "Some fixed string"); + tree.AddAttribute(1, "MyByte", (byte)123); + tree.AddAttribute(1, "MyInt", int.MaxValue); + tree.AddAttribute(1, "MyLong", long.MaxValue); + tree.AddAttribute(1, "MyBool", true); + tree.AddAttribute(1, "MyFloat", float.MaxValue); + tree.AddAttribute(1, "MyDouble", double.MaxValue); + tree.AddAttribute(1, "MyDecimal", decimal.MinusOne); + tree.AddAttribute(1, "MyDate", dateTimeWillNotChange); + tree.AddAttribute(1, "MyFragment", fragmentWillNotChange); // Treat fragments as primitive + tree.CloseComponent(); + } + + RenderTreeDiffBuilder.ComputeDiff(renderer, new RenderBatchBuilder(), 0, new RenderTreeBuilder(renderer).GetFrames(), oldTree.GetFrames()); + var originalComponentInstance = (CaptureSetParametersComponent)oldTree.GetFrames().Array[0].Component; + Assert.Equal(1, originalComponentInstance.SetParametersCallCount); + + // Act + var renderBatch = GetRenderedBatch(); + var newComponentInstance = (CaptureSetParametersComponent)oldTree.GetFrames().Array[0].Component; + + // Assert + Assert.Same(originalComponentInstance, newComponentInstance); + Assert.Equal(1, originalComponentInstance.SetParametersCallCount); // Received no further parameter change notification + } + [Fact] public void QueuesRemovedChildComponentsForDisposal() { @@ -915,6 +953,20 @@ namespace Microsoft.AspNetCore.Blazor.Test } } + private class CaptureSetParametersComponent : IComponent + { + public int SetParametersCallCount { get; private set; } + + public void Init(RenderHandle renderHandle) + { + } + + public void SetParameters(ParameterCollection parameters) + { + SetParametersCallCount++; + } + } + private class DisposableComponent : IComponent, IDisposable { public int DisposalCount { get; private set; } diff --git a/test/Microsoft.AspNetCore.Blazor.Test/RendererTest.cs b/test/Microsoft.AspNetCore.Blazor.Test/RendererTest.cs index ffeebe7eca..aa2be87106 100644 --- a/test/Microsoft.AspNetCore.Blazor.Test/RendererTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Test/RendererTest.cs @@ -341,7 +341,7 @@ namespace Microsoft.AspNetCore.Blazor.Test Assert.Equal(0, edit.ReferenceFrameIndex); }); AssertFrame.Text(batch.ReferenceFrames[0], "Modified message"); - Assert.Empty(batch.DiffsByComponentId[nestedComponentFrame.ComponentId].Single().Edits); + Assert.False(batch.DiffsByComponentId.ContainsKey(nestedComponentFrame.ComponentId)); } [Fact]