Aspnetcore: [Blazor] Improve BuildRenderTree performance by allowing the app developer to state unequivocally that specific state has not changed

[mrpmorris]

Problem

The current ChangeDetection.MayHaveChanged method indicates a BuildRenderTree is required if

1: The type of the old / new is different
2: Or if it is not a known immutable type
3: Or if the value has changed

https://github.com/dotnet/aspnetcore/blob/6be5f2072a4a77b654c15a3dcc432964a853b342/src/Components/Components/src/ChangeDetection.cs#L22

The problem with this is that all objects will be considered possibly changed if they are not a known immutable type.

I now use record types extensively throughout my state, and the current logic will assume they mave have changed - this will result in unnecessary calls to BuildRenderTree.

Request

Make ChangeDetection public and add a global hook that lets me
A: Not specify that I know if a value has changed or not
B: Indicate I know that a deep state comparison of an object is unchanged
C: Indicate I know an object has definitely changed (overrides any handler that says it is unchanged)

This will not only make it unnecessary to override ShouldRender in our app components that render objects we know haven't changed, but will also ensure that 3rd part components are aware that they don't need to re-render too.

Suggested changes

Here are some suggested changes to the ChangeDetection class

using System;

namespace Microsoft.AspNetCore.Components
{
    public static class ChangeDetection
    {
        public static event EventHandler<ChangeDetectionEventArgs> DeepCompareValues;

        public static bool MayHaveChanged<T1, T2>(T1 oldValue, T2 newValue)
        {
            Type? oldType = oldValue?.GetType();
            Type? newType = newValue?.GetType();

            if (oldType is null && newType is null)
                return false; // Both are null, so the same

            if (oldType != newType)
                return true; // Either one is null, or neither are null but are different types

            EventHandler<ChangeDetectionEventArgs> compareValues = DeepCompareValues;
            if (compareValues != null)
            {
                var args = new ChangeDetectionEventArgs(oldValue, newValue);
                compareValues.Invoke(null, args);
                switch (args.Result)
                {
                    case ChangeDetectionResult.Changed:
                        return true;
                    case ChangeDetectionResult.Unchanged:
                        return false;
                    case ChangeDetectionResult.Unknown:
                        break;
                    default:
                        throw new NotImplementedException(args.Result.ToString());
                }
            }


            if (!IsKnownImmutableType(oldType) // Mutable types might be different
                    || !oldValue.Equals(newValue)) // Is mutable but not equal
            {
                return true;
            }

            // By now we know either both are null, or they are the same immutable type
            // and ThatType::Equals says the two values are equal.
            return false;
        }

        // 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 static bool IsKnownImmutableType(Type type)
                => type.IsPrimitive
                || type == typeof(string)
                || type == typeof(DateTime)
                || type == typeof(Type)
                || type == typeof(decimal);
    }

    public class ChangeDetectionEventArgs
    {
        public object OldValue { get; }
        public object NewValue { get; }
        public ChangeDetectionResult Result { get; private set; }

        public ChangeDetectionEventArgs(object oldValue, object newValue)
        {
            OldValue = oldValue;
            NewValue = newValue;
            Result = ChangeDetectionResult.Unknown;
        }

        public void IsUnchanged()
        {
            if (Result == ChangeDetectionResult.Unknown)
                Result = ChangeDetectionResult.Unchanged;
        }

        public void HasChanged()
        {
            Result = ChangeDetectionResult.Changed;
        }
    }

    public enum ChangeDetectionResult
    {
        Unknown,
        Unchanged,
        Changed
    }
}

Example use

public record Person(string FirstName, string LastName);

class Program
{
    static void Main(string[] args)
    {
        // Returns false
        WriteHasChanged(42, 42);

        var originalPerson = new Person("Peter", "Morris");
        var newPerson = originalPerson with { LastName = "Smith" };

        // Returns true because they are definitely different
        WriteHasChanged(originalPerson, newPerson);

        // Returns true. Even though the values have the same values the instances are different
        newPerson = originalPerson with { LastName = "Morris" };
        WriteHasChanged(originalPerson, newPerson);

        // Add my own comparer so that when the object is a Person then I can
        // use record comparison to indicate that I know it has not changed and so doesn't need to render
        ChangeDetection.DeepCompareValues += (_, args) =>
        {
            if (args.OldValue is Person)
            {
                if (args.OldValue.Equals(args.NewValue))
                    args.IsUnchanged();
            }
        };

        // Returns false. Even though these are different instances and not immutable types, my custom
        // handler has told Blazor that I am certain the state has not changed.
        WriteHasChanged(originalPerson, newPerson);
    }

    private static void WriteHasChanged<T>(T oldValue, T newValue)
    {
        bool result = ChangeDetection.MayHaveChanged(oldValue, newValue);
        Console.WriteLine($"MayHaveChanged({oldValue}, {newValue}) = {result}");
    }
}

[mrpmorris]

I've updated the ticket to refer explicitly to record types instead of just IEquatable, as the IEquatable is obviously about identity equality rather than state equality.

[SteveSandersonMS]

Unfortunately, record types are not deeply immutable. They can contain properties that are themselves mutable.

The runtime would have to walk all chains of reachable properties to see if the transitive closure is immutable (including reflecting over all private fields in the graph), which is not great. It would lead to traps like ‘adding a new private field of type ‘object’ to a seemingly unrelated type suddenly causes widespread changes to rendering characteristics’. We try to avoid designing in traps like that.

[SteveSandersonMS]

What we have considered is an attribute like [TreatAsDeeplyImmutable] you could put on any type if you wanted the system to behave as if it was deeply immutable (whether or not it actually is).

[mrpmorris]

That would be good if you could get it into the DataAnnotations library. I wouldn't want a Blazor dependancy in my state project.

What about a global hook in Blazor we can set that the ChangeDetection can ask if two values are deep equal?

[SteveSandersonMS]

Unfortunately it’s meaningless to talk about ‘two values’ being equal because typically the old and new values point to the same object instance. The question to be answered is ‘has it mutated?’.

If you really want to control the ‘should this component render now’ logic you can do so by overriding ShouldRender.

[mrpmorris]

I'd have to do that in every component, and would be unable to in third party components.

If I could do it globally then I could check object instances on my records and it'd work for every Blazor component regardless of who wrote them.

I think it'd be great to give this control to the app developer. Only they truly know if their state is definitely unchanged.

Using this feature and proper use of immutable records would reduce my rendering by about 99%. That's a brilliant gain!

I don't see any down sides.

[mrpmorris]

@SteveSandersonMS I have update the title + description to reflect a better approach resulting from our conversation here.

I have also included the necessary code + an example application source.

[Joe4evr]

@mrpmorris In addition to records not being deeply immutable, you've made the (unfortunately very common) mistaken assumption that records are always immutable in the first place. So despite your effort of using records, the render logic still won't be able to get around having to assume that instances may have changed, because at the end of the day records are really ordinary classes just with more auto-generated members.

[mrpmorris]

@Joe4evr Indeed. Ultimately only the app developer can be expected to know if their types will remain unchanged, not component vendors.

That's why I updated the request to allow a fixed point the dev can use to tell Blazor the state is unchanged.

[kiyote]

I'm not sure I understand the pushback on this. If the framework already incorporates one performance turnstile, what is the objection to adding another, especially for circumstances where clearly only the developer can say for sure?

[SteveSandersonMS]

@kiyote There's no pushback 😄 The framework already has ShouldRender to give the developer absolute control over this.

In this issue we are tentatively considering a built-in mechanism for denoting a certain custom type as immutable (e.g., a [TreatAsImmutable] attribute). Or, if it wasn't an attribute, it could be done via some global callback like bool IsMutable(object instance) or bool IsMutable(Type type), or perhaps some registry like Components.RegisterImmutableType<T>().

When it comes to your own components, you can already implement any of the above mechanisms yourself by overriding ShouldRender.

[mrpmorris]

Hi @SteveSandersonMS I don't see pushback :)

Whatever it is you choose to do, please don't make it so I have to reference Blazor from the library that holds my state as it will be UI agnostic.

Or, at least don't make it the only way. A global hook that also looks for attributes would obviously do just as nicely :)

[toddlang]

Honestly, I'd prefer an approach that let me inject my own code to do the evaluation, rather than an annotation. I already have an analyzer that checks that things I've tagged [Immutable] are actually immutable, so I could just leverage that with "does it have the tag, or is a known immutable type?" in some routine, rather than having re-code things to utilize a new attribute.

Upon reflection, registering the types in an "immutable map" would also be quite reasonable. And I suppose it would _feel_ more like DI as well.

[mrpmorris]

@SteveSandersonMS I just spotted this attribute. Could you use this, or is it in an assembly that isn't referenced?

https://docs.microsoft.com/en-us/dotnet/api/system.componentmodel.immutableobjectattribute?view=netcore-3.1

[SteveSandersonMS]

@mrpmorris Maybe we could. Blazor applications already reference System.ComponentModel.Primitives by default, so it is available. One cause for pause is that the docs for it are quite clear that it's only intended for use in design-time tooling:

As such, this property is used only at design time. [[source](https://docs.microsoft.com/en-us/dotnet/api/system.componentmodel.immutableobjectattribute?view=netcore-3.1)]

Giving it meaning at runtime would not be consistent with that, so we'd at least have to bring it as a proposal to the base class library owners and ask whether they are happy to generalise its meaning, change the guidance, etc.