Fix the IsKnownImmutableType method to return true for enum types

[alix-tlse]

Enum parameters

Fix the IsKnownImmutableType method to return true for enum types

Description

If the type of a component parameter is an enum, the SetParameter method is called everytime, even if the parameter don't change. We add here the the handle for enum types.

Fixes #42891

[ghost]

Thanks for your PR, @alix-tlse. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[javiercn]

/cc: @SteveSandersonMS In case you have any concerns (it strikes me as wrong that we forgot to do add enums, but it might have been an oversight)

[SteveSandersonMS]

In case you have any concerns

No concerns - this seems fine to me.

In general IsKnownImmutableType has to strike a balance between understanding enough types while being fast enough to run. This is so perf-critical within rendering that each extra "or" clause is consequential (and we don't want to get into even heavier techniques like doing dictionary lookups). Given how commonly used enums are, I think they warrant the one extra check here, at least assuming that on Mono the IsEnum flag is no more expensive to evaluate than something like an auto-property getter.

it strikes me as wrong

Not really wrong or an oversight - it's a subjective tradeoff. False negatives are fine in this case, false positives are not, and slow anything is not. It's intentional to limit the set of types we match here to minimize the evaluation cost. Enums are fairly easy to make a case for so I'd be fine with expanding the set to include those.

Of course, we'd also need tests to be able to merge this PR :)

[campersau]

@alix-tlse here is an example for a test: https://github.com/dotnet/aspnetcore/commit/74501d9a4521d3a1320054d6bc89b9a3eb97f804 (similar to https://github.com/dotnet/aspnetcore/pull/32753) There are currently tabs and not spaces in this fix.

[alix-tlse]

Hello,

I did the following tests to compare the method call with the "IsEnum" and without.

The main difference, for types that do not meet any conditions, is between "TestV6_Obj" (Without IsEnum) and the "TestV7_Obj" (With IsEnum), The execution time increases from 12 ns to 24ns. (Console context)

I also did tests on the same computer in WASM context, for 1000 calls we lose 1ms. We talk about 1000 calls to not immutable types, so for me it seems acceptable, but I let you judge ;-)

The code :

namespace Benchmarks
{
    public class BenchmarkChangeDetection
    {

        private static Type _intType = typeof(int);
        private static Type _objectType = typeof(object);

        [Benchmark]
        public void TestV5_Int()
        {
            _ = IsKnownImmutableTypeV5(_intType);
        }

        [Benchmark]
        public void TestV6_Int()
        {
            _ = IsKnownImmutableTypeV6(_intType);
        }

        [Benchmark]
        public void TestV7_Int()
        {
            _ = IsKnownImmutableTypeV7(_intType);
        }

        [Benchmark]
        public void TestV5_Obj()
        {
            _ = IsKnownImmutableTypeV5(_objectType);
        }

        [Benchmark]
        public void TestV6_Obj()
        {
            _ = IsKnownImmutableTypeV6(_objectType);
        }

        [Benchmark]
        public void TestV7_Obj()
        {
            _ = IsKnownImmutableTypeV7(_objectType);
        }

        private static bool IsKnownImmutableTypeV5(Type type)
        {
            return type.IsPrimitive
                    || type == typeof(string)
                    || type == typeof(DateTime)
                    || type == typeof(Type)
                    || type == typeof(decimal);
        }
        
        private static bool IsKnownImmutableTypeV6(Type type)
        {
            return type.IsPrimitive
                    || type == typeof(string)
                    || type == typeof(DateTime)
                    || type == typeof(Type)
                    || type == typeof(decimal)
                    || type == typeof(Guid);
        }

        private static bool IsKnownImmutableTypeV7(Type type)
        {
            return type.IsPrimitive
                || type == typeof(string)
                || type == typeof(DateTime)
                || type == typeof(Type)
                || type == typeof(decimal)
                || type == typeof(Guid)
                || type.IsEnum;
        }
    }
}

The result :

// * Summary *

BenchmarkDotNet=v0.13.2, OS=Windows 10 (10.0.19041.1415/2004/May2020Update/20H1) 11th Gen Intel Core i7-1165G7 2.80GHz, 1 CPU, 8 logical and 4 physical cores .NET SDK=6.0.400 [Host] : .NET 6.0.8 (6.0.822.36306), X64 RyuJIT AVX2 DefaultJob : .NET 6.0.8 (6.0.822.36306), X64 RyuJIT AVX2

Method	Mean	Error	StdDev	Median
TestV5_Int	5.408 ns	0.2276 ns	0.6709 ns	5.143 ns
TestV6_Int	5.067 ns	0.2125 ns	0.6266 ns	4.749 ns
TestV7_Int	5.119 ns	0.1313 ns	0.1289 ns	5.154 ns
TestV5_Obj	10.145 ns	0.4392 ns	1.2950 ns	9.434 ns
TestV6_Obj	11.798 ns	0.4514 ns	1.3309 ns	11.172 ns
TestV7_Obj	23.668 ns	0.8461 ns	2.4949 ns	22.864 ns

// * Warnings * MultimodalDistribution BenchmarkChangeDetection.TestV5_Int: Default -> It seems that the distribution can have several modes (mValue = 3.19) BenchmarkChangeDetection.TestV5_Obj: Default -> It seems that the distribution is bimodal (mValue = 3.25) BenchmarkChangeDetection.TestV6_Obj: Default -> It seems that the distribution is bimodal (mValue = 3.24) BenchmarkChangeDetection.TestV7_Obj: Default -> It seems that the distribution is bimodal (mValue = 3.79)

// * Hints * Outliers BenchmarkChangeDetection.TestV7_Int: Default -> 4 outliers were removed, 5 outliers were detected (6.36 ns, 7.96 ns..8.29 ns)

// * Legends * Mean : Arithmetic mean of all measurements Error : Half of 99.9% confidence interval StdDev : Standard deviation of all measurements Median : Value separating the higher half of all measurements (50th percentile) 1 ns : 1 Nanosecond (0.000000001 sec)

// ***** BenchmarkRunner: End ***** Run time: 00:08:56 (536.06 sec), executed benchmarks: 6

[SteveSandersonMS]

Thanks very much @alix-tlse for doing this benchmarking! That's super useful.

Doubling the execution time isn't ideal, and we can see from your numbers that each condition here has an obvious effect on the cost. It's not really an issue on CoreCLR since these times measured in ns are unlikely to multiply up to anything problematic, even in a very heavy rendering workload that's processing tens or hundreds of thousands of parameters. However it is something we should think carefully about for WebAssembly.

Since you have this benchmarking infrastructure already set up, it would be really interesting to know if there are any simple techniques we could use to get the benefits of checking more types without paying extra costs. For example, is there any simple and practical way to cache the lookups in a Dictionary<Type, bool>, or do the dictionary lookups themselves cost more than doing a series of "if/else" tests? If you're interested in trying that out that would be great, but if you'd rather not that's totally fine too!

[alix-tlse]

I tried the approach with a cache ("V7_Cache_..." in the sumary), but it slows down the processing for primitive types. However I found another approach with the "Type.GetTypeCode" method and the results are very good compared to version 6.0, twice as fast for primitive types, 5 times faster for other types !

The only change in behavior is that we support enumeration and we don't support IntPtr and UIntPtr anymore, but we could add them if needed...

BenchmarkDotNet=v0.13.2, OS=Windows 10 (10.0.19041.1415/2004/May2020Update/20H1) 11th Gen Intel Core i7-1165G7 2.80GHz, 1 CPU, 8 logical and 4 physical cores .NET SDK=6.0.400 [Host] : .NET 6.0.8 (6.0.822.36306), X64 RyuJIT AVX2 DefaultJob : .NET 6.0.8 (6.0.822.36306), X64 RyuJIT AVX2

Method	Mean	Error	StdDev	Median
🔵 V6_Int	4.941 ns	0.1850 ns	0.5454 ns	4.647 ns
🟢 V7_Code_Int	2.411 ns	0.1248 ns	0.3680 ns	2.257 ns
V7_IsEnum_Int	5.001 ns	0.2209 ns	0.6514 ns	4.754 ns
V7_Cache_Int	17.810 ns	0.6703 ns	1.9765 ns	16.866 ns
🔵 V6_Guid	12.076 ns	0.4234 ns	1.2485 ns	11.539 ns
🟢 V7_Code_Guid	2.218 ns	0.1213 ns	0.3576 ns	2.013 ns
V7_IsEnum_Guid	12.294 ns	0.3868 ns	1.1404 ns	11.919 ns
V7_Cache_Guid	18.122 ns	0.7011 ns	2.0450 ns	17.330 ns
🔵 V6_Obj	11.810 ns	0.3975 ns	1.1720 ns	11.181 ns
🟢 V7_Code_Obj	2.217 ns	0.1251 ns	0.3689 ns	2.018 ns
🔴 V7_Code_ObjExp	7.592 ns	0.2748 ns	0.8102 ns	7.299 ns
V7_IsEnum_Obj	23.869 ns	0.8464 ns	2.4955 ns	22.602 ns
V7_Cache_Obj	18.767 ns	0.6755 ns	1.9918 ns	17.966 ns

TypeCode for known types :

private enum MyEnum
{
    A,
    B
}

private static void TestTypeCodes()
{
    // Primitive types
    Console.WriteLine($"bool: {Type.GetTypeCode(typeof(bool))}");                       // bool: Boolean       
    Console.WriteLine($"byte: {Type.GetTypeCode(typeof(byte))}");                       // byte: Byte       
    Console.WriteLine($"sbyte: {Type.GetTypeCode(typeof(sbyte))}");                     // sbyte: SByte       
    Console.WriteLine($"short: {Type.GetTypeCode(typeof(short))}");                     // short: Int16      
    Console.WriteLine($"ushort: {Type.GetTypeCode(typeof(ushort))}");                   // ushort: UInt16       
    Console.WriteLine($"int: {Type.GetTypeCode(typeof(int))}");                         // int: Int32       
    Console.WriteLine($"uint: {Type.GetTypeCode(typeof(uint))}");                       // uint: UInt32       
    Console.WriteLine($"long: {Type.GetTypeCode(typeof(long))}");                       // long: Int64       
    Console.WriteLine($"ulong: {Type.GetTypeCode(typeof(ulong))}");                     // ulong: UInt64       
    Console.WriteLine($"IntPtr: {Type.GetTypeCode(typeof(IntPtr))}");                   // IntPtr: Object ⚠️       
    Console.WriteLine($"UIntPtr: {Type.GetTypeCode(typeof(UIntPtr))}");                 // UIntPtr: Object ⚠️      
    Console.WriteLine($"char: {Type.GetTypeCode(typeof(char))}");                       // char: Char       
    Console.WriteLine($"double: {Type.GetTypeCode(typeof(double))}");                   // double: Double
    // Other types
    Console.WriteLine($"string: {Type.GetTypeCode(typeof(string))}");                   // string: String       
    Console.WriteLine($"DateTime: {Type.GetTypeCode(typeof(DateTime))}");               // DateTime: DateTime       
    Console.WriteLine($"Type: {Type.GetTypeCode(typeof(Type))}");                       // Type: Object ⚠️      
    Console.WriteLine($"decimal: {Type.GetTypeCode(typeof(decimal))}");                 // decimal: Decimal       
    Console.WriteLine($"Guid: {Type.GetTypeCode(typeof(Guid))}");                       // Guid: Object ⚠️      
    Console.WriteLine($"Enum: {Type.GetTypeCode(typeof(MyEnum))}");                     // Enum: Int32 🔥      
}

The full code :

using BenchmarkDotNet.Attributes;

namespace Benchmarks
{
    public class BenchmarkChangeDetection
    {

        private static Dictionary<Type, bool> _cache = new();

        private static Type _intType = typeof(int);
        private static Type _guidType = typeof(Guid);
        private static Type _objectType = typeof(object);


        #region int tests

        [Benchmark]
        public void V6_Int()
        {
            _ = IsKnownImmutableTypeV6(_intType);
        }

        [Benchmark]
        public void V7_IsEnum_Int()
        {
            _ = IsKnownImmutableTypeV7Enum(_intType);
        }

        [Benchmark]
        public void V7_Cache_Int()
        {
            _ = IsKnownImmutableTypeV7Cache(_intType);
        }

        [Benchmark]
        public void V7_Code_Int()
        {
            _ = IsKnownImmutableTypeV7Code(_intType);
        }

        #endregion

        #region guid tests

        [Benchmark]
        public void V6_Guid()
        {
            _ = IsKnownImmutableTypeV6(_guidType);
        }

        [Benchmark]
        public void V7_IsEnum_Guid()
        {
            _ = IsKnownImmutableTypeV7Enum(_guidType);
        }

        [Benchmark]
        public void V7_Cache_Guid()
        {
            _ = IsKnownImmutableTypeV7Cache(_guidType);
        }

        [Benchmark]
        public void V7_Code_Guid()
        {
            _ = IsKnownImmutableTypeV7Code(_guidType);
        }

        #endregion

        #region object type tests

        [Benchmark]
        public void V6_Obj()
        {
            _ = IsKnownImmutableTypeV6(_objectType);
        }

        [Benchmark]
        public void V7_IsEnum_Obj()
        {
            _ = IsKnownImmutableTypeV7Enum(_objectType);
        }

        [Benchmark]
        public void V7_Cache_Obj()
        {
            _ = IsKnownImmutableTypeV7Cache(_objectType);
        }

        [Benchmark]
        public void V7_Code_Obj()
        {
            _ = IsKnownImmutableTypeV7Code(_objectType);
        }

        #endregion

        #region switch expression test

        [Benchmark]
        public void V7_Code_ObjExp()
        {
            _ = IsKnownImmutableTypeV7CodeExp(_objectType);
        }

        #endregion

        private static bool IsKnownImmutableTypeV6(Type type)
        {
            return type.IsPrimitive
                    || type == typeof(string)
                    || type == typeof(DateTime)
                    || type == typeof(Type)
                    || type == typeof(decimal)
                    || type == typeof(Guid);
        }

        private static bool IsKnownImmutableTypeV7Enum(Type type)
        {
            return type.IsPrimitive
                || type == typeof(string)
                || type == typeof(DateTime)
                || type == typeof(Type)
                || type == typeof(decimal)
                || type == typeof(Guid)
                || type.IsEnum;
        }

        private static bool IsKnownImmutableTypeV7Cache(Type type)
        {
            if (_cache.TryGetValue(type, out bool result))
            {
                return result;
            }
            else
            {
                result = IsKnownImmutableTypeV7Enum(type);
                _cache.Add(type, result);
                return result;
            }
        }

        private static bool IsKnownImmutableTypeV7Code(Type type)
        {
#pragma warning disable IDE0066 // Convert to switch expression is 3 times slower
            switch (Type.GetTypeCode(type))
            {
                case TypeCode.Object:
                    return type == typeof(Type) || type == typeof(Guid);
                default:
                    return true;

            }
#pragma warning restore IDE0066
        }

        private static bool IsKnownImmutableTypeV7CodeExp(Type type)
        {
            return Type.GetTypeCode(type) switch
            {
                TypeCode.Object => type == typeof(Type) || type == typeof(Guid),
                _ => true,
            };
        }
    }
}

[alix-tlse]

The final version :

        private static bool IsKnownImmutableTypeV7CodeIf(Type type)
        {
            return Type.GetTypeCode(type) != TypeCode.Object 
                || type == typeof(Type) 
                || type == typeof(Guid);
        }

// * Summary *

BenchmarkDotNet=v0.13.2, OS=Windows 10 (10.0.19041.1415/2004/May2020Update/20H1) 11th Gen Intel Core i7-1165G7 2.80GHz, 1 CPU, 8 logical and 4 physical cores .NET SDK=6.0.400 [Host] : .NET 6.0.8 (6.0.822.36306), X64 RyuJIT AVX2 DefaultJob : .NET 6.0.8 (6.0.822.36306), X64 RyuJIT AVX2

Method	Mean	Error	StdDev	Median
🔵 V6_Int	4.371 ns	0.1391 ns	0.4102 ns	4.215 ns
🟢 V7_Code_IntIf	2.214 ns	0.0978 ns	0.2884 ns	2.239 ns
⚫ V7_Code_Int	2.539 ns	0.1038 ns	0.3044 ns	2.486 ns
🔵 V6_Guid	11.524 ns	0.3466 ns	1.0220 ns	11.150 ns
🟢 V7_Code_GuidIf	2.007 ns	0.0970 ns	0.2861 ns	2.125 ns
⚫ V7_Code_Guid	1.992 ns	0.0779 ns	0.2297 ns	1.888 ns
🔵 V6_Obj	11.064 ns	0.3096 ns	0.9130 ns	10.610 ns
🟢 V7_Code_ObjIf	1.869 ns	0.0926 ns	0.2730 ns	1.720 ns
⚫ V7_Code_Obj	1.986 ns	0.0892 ns	0.2629 ns	1.832 ns

• 🔵 V6_... : The .NET Core 6 version
• 🟢 V7_Code_...If : New version
• ⚫ V7_Code_... : Old version with a switch

I will update my PR in this way...

[SteveSandersonMS]

Thanks very much, @alix-tlse! This looks really promising!

The benchmark numbers you have clearly show a big improvement on CoreCLR. However the more critical scenario is WebAssembly since that's the case where people are more likely to test the limits of rendering performance. Would it be possible to verify the gains/losses when running in a browser?

With the final algorithm as given, there is I guess some risk that future versions of .NET add new TypeCode entries corresponding to mutable types, and then we'd falsely claim they are immutable. One way we could avoid this risk would be to invert the logic a bit based on the numeric values of TypeCode. Currently all the values in the range 2-16 are used and are for immutable types, so any newly added mutable types would have to be outside that range. So what about something like this?

static bool IsKnownImmutableTypeV8(Type type)
{
    return Type.GetTypeCode(type) switch
    {
        // This range of typecode values has no gaps and is known only to represent immutable primitive types (e.g., int, bool)
        // See https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/TypeCode.cs
        >= TypeCode.DBNull and <= TypeCode.DateTime => true,

        // All nonprimitive types will fall into this branch, plus a few rarely-seen primitives. It's OK to treat the rarely-seen ones
        // (e.g., IntPtr) as possibly-mutable because it's more important to optimize for speed and only detect common types
        _ => type == typeof(string) || type == typeof(Type) || type == typeof(Guid)
    };
}

I know this will perform one extra comparison than your optimized version, but the future-proofness would probably justify this if it's still a lot faster than the code we already ship! That is, unless we have some way to know that the TypeCode enum will never be expanded.

Finally, to be merge the PR, it would be great to expand the SkipsUpdatingParametersOnChildComponentsIfAllAreDefinitelyImmutableAndUnchanged test in RenderTreeDiffBuilderTest.cs to cover the new things now recognized (like uint etc.).

[alix-tlse]

Continuation of the performance tests :

// * Summary *

BenchmarkDotNet=v0.13.2, OS=Windows 10 (10.0.19041.1415/2004/May2020Update/20H1) 11th Gen Intel Core i7-1165G7 2.80GHz, 1 CPU, 8 logical and 4 physical cores .NET SDK=6.0.400 [Host] : .NET 6.0.8 (6.0.822.36306), X64 RyuJIT AVX2 DefaultJob : .NET 6.0.8 (6.0.822.36306), X64 RyuJIT AVX2

Method	Mean	Error	StdDev	Median
V6_Int	4.829 ns	0.2206 ns	0.6470 ns	4.495 ns
V7_Stv_Int	4.442 ns	0.1656 ns	0.4857 ns	4.275 ns
V7_Int	2.585 ns	0.1685 ns	0.4969 ns	2.511 ns
V6_Guid	11.570 ns	0.4306 ns	1.2697 ns	10.959 ns
V7_Stv_Guid	8.736 ns	0.3597 ns	1.0551 ns	8.362 ns
V7_Guid	2.187 ns	0.1472 ns	0.4341 ns	2.005 ns
V6_Obj	15.038 ns	1.0433 ns	3.0435 ns	15.789 ns
V7_Stv_Obj	12.048 ns	0.6634 ns	1.9560 ns	11.768 ns
V7_Obj	3.151 ns	0.2913 ns	0.8311 ns	2.991 ns

• V6_... : The .NET Core 6 version
• V7_Stv_... : The IsKnownImmutableTypeV8 proposed above
• V7_... : The current PR method with only test on TypeCode.Object value.

In the WASM context the differences are not as important :

Method	Duration
V6_Obj	1440 ns
V7_Stv_Obj	751 ns
V7_Obj	703 ns

"there is I guess some risk that future versions of .NET add new TypeCode entries corresponding to mutable types"

With all my respect, this enumeration didn't change since 22 years and we can't predict, in the case it will change, if the type added will be mutable or not... (DateTimeOffset, Guid, ...). So (for me 😉), the best choice is the simpliest and quickest method :

private static bool IsKnownImmutableTypeV7CodeIf(Type type)
{
    return Type.GetTypeCode(type) != TypeCode.Object 
        || type == typeof(Type) 
        || type == typeof(Guid);
}

The "SkipsUpdatingParametersOnChildComponentsIfAllAreDefinitelyImmutableAndUnchanged" has been updated !

[dnfadmin]

All CLA requirements met.

[SteveSandersonMS]

OK that's great, thanks for the updated benchmark numbers and test!

With all my respect, this enumeration didn't change since 22 years

OK, that does sound reasonable given the history. Perhaps we could add a comment there saying:

// This logic assumes that no new System.TypeCode enum entries have been declared since 7.0, or at least that any new ones
// represent immutable types. If System.TypeCode changes, review this logic to ensure it is still correct.

I know people probably won't remember to check this in the future, but at least it leaves a record of why we reasoned that the logic is valid and the circumstances under which we know it would need to change.

That's probably all that's needed for us to be able to merge this! Note that it will ship in .NET 8, as .NET 7 is already into RC2.

[alix-tlse]

OK, thanks to all of you @SteveSandersonMS, @javiercn and @campersau ! It was a nice experience, I will wait patiently for the next year release.

[alix-tlse]

Hello @SteveSandersonMS , it's me again with a question :-)

"Type" parameters

I think the tests did not pass during my last pull request because i add a test for parameter of type "Type" but the existing condition was not valid :

... || type == typeof(Type)

"Type" is an abstract type, this condition will always return false. It should have been done (source) :

typeof(Type).IsAssignableFrom(type)`.

I tested to pass a parameter of type "Type" with the .NET 6, the method SetParameterAsync is called systematically.

In my opinion, that it is not necessary to have a particular treatment on the "Type" types because they are not much used. If we have to pass a type to the component, we will rather use a generic component with the use of @typeparam for example...

The "EventCallBack" parameters

Taking this a step further, the "EventCallBack" and "EventCallBack<>" parameters are very much used and are immutable. This could avoid many SetParameterAsync and re-render components for declaration like this :

<MyButton Kind="ButtonKind.Secondary" OnClick="()=>currentCount++" Text="Click Me !" />

As you can see in the following benchmarks with the "IsKnownImmutableTypeV8d" method, we can handle the case still being faster than on the current version.

WebAssembly Benchmarks

Method	Enum*	Type*	EventCallBack*	int**	Guid**	object**	List<int>**	Comment
...V6	no	no	no	740	1 573	1 735	1 702	.NET 6/7 version
...V8a	yes	yes	no	430	485	818	875
...V8b	yes	no	no	441	507	623	629
...V8c	yes	yes	yes	453	502	1 503	1 518
...V8d	yes	no	yes	441	491	1 232	1 269	The choosen one
...V8e	yes	no	yes	459	515	1 255	2 223

* Supported immutable ** Type of parameter tested (Result in Nanosecondes)

The question

Can I modify my pull request to remove the "Type" test and add "EventCallBack" management with the following version of the method ?

// 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.
// This logic assumes that no new System.TypeCode enum entries have been declared since 7.0, or at least that any new ones
// represent immutable types. If System.TypeCode changes, review this logic to ensure it is still correct.
// Supported immutable types : bool, byte, sbyte, short, ushort, int, uint, long, ulong, char, double,
//                             string, DateTime, decimal, Guid, Enum, EventCallBack, EventCallBack<>.
// For performance reasons, the following immutable types are not supported: IntPtr, UIntPtr, Type.
private static bool IsKnownImmutableType(Type type)
	=> Type.GetTypeCode(type) != TypeCode.Object
            || type == typeof(Guid)
            || typeof(IEventCallBack).IsAssignableFrom(type);
}

Test Method

        private static Type _runtimeType = typeof(Type).GetType();

        private static bool IsKnownImmutableTypeV6(Type type)
        {
            return type.IsPrimitive
                    || type == typeof(string)
                    || type == typeof(DateTime)
                    || type == typeof(Type)
                    || type == typeof(decimal)
                    || type == typeof(Guid);
        }

        private static bool IsKnownImmutableTypeV8a(Type type)
        {
            return Type.GetTypeCode(type) != TypeCode.Object
                || type == typeof(Guid)
                || type == _runtimeType;
        }

        private static bool IsKnownImmutableTypeV8b(Type type)
        {
            return Type.GetTypeCode(type) != TypeCode.Object
                || type == typeof(Guid);
        }

        private static bool IsKnownImmutableTypeV8c(Type type)
        {
            return Type.GetTypeCode(type) != TypeCode.Object
                || type == typeof(Guid)
                || type == _runtimeType
                || typeof(IEventCallBack).IsAssignableFrom(type);
        }

        private static bool IsKnownImmutableTypeV8d(Type type)
        {
            return Type.GetTypeCode(type) != TypeCode.Object
                || type == typeof(Guid)
                || typeof(IEventCallBack).IsAssignableFrom(type);
        }

        private static bool IsKnownImmutableTypeV8e(Type type)
        {
            if (Type.GetTypeCode(type) != TypeCode.Object || type == typeof(Guid))
            {
                return true;
            }
            if (type.IsGenericType)
            {
                return type.GetGenericTypeDefinition() == typeof(EventCallBack<>);
            }
            else
            {
                return type == typeof(EventCallBack);
            }
        }

[alperenbelgic]

I am wondering if somehow we can override and include our own logic in the future, case by case basis. While I see performance considerations are a crucial aspect, I thought record types would be reasonable for this too as they are compared as values. I can achieve it by overriding ShouldRender, but I am wondering if it would be more convenient this way.

[alix-tlse]

I am wondering if somehow we can override and include our own logic in the future, case by case basis. While I see performance considerations are a crucial aspect, I thought record types would be reasonable for this too as they are compared as values. I can achieve it by overriding ShouldRender, but I am wondering if it would be more convenient this way.

There does not seem to be a clean way to detect a record type: see here

But a way to add more flexibility would be to create an interface called "IImmutableParameter" for example.

To make IEventCallBack inherit from IImmutableParameter.

Developers could then declare records like this:

        public record Person : IImmutableParameter
        {
            public string FirstName { get; }
            public string LastName { get; }
        };

By modifying the "IsKnownImmutableType" method like this :

private static bool IsKnownImmutableType(Type type)
	=> Type.GetTypeCode(type) != TypeCode.Object
            || type == typeof(Guid)
            || typeof(IImmutableParameter).IsAssignableFrom(type);
}

Edit : This approach (even if it offers developers the possibility to bypass the SetParameter in some optimization cases) is little bit hacky,the best way would be to had a "IsImmutable" property on System.Type...

[alperenbelgic]

I am wondering if somehow we can override and include our own logic in the future, case by case basis. While I see performance considerations are a crucial aspect, I thought record types would be reasonable for this too as they are compared as values. I can achieve it by overriding ShouldRender, but I am wondering if it would be more convenient this way.

There does not seem to be a clean way to detect a record type: see here

But a way to add more flexibility would be to create an interface called "IImmutableParameter" for example.

To make IEventCallBack inherit from IImmutableParameter.

Developers could then declare records like this:

        public record Person : IImmutableParameter
        {
            public string FirstName { get; }
            public string LastName { get; }
        };

By modifying the "IsKnownImmutableType" method like this :

private static bool IsKnownImmutableType(Type type)
	=> Type.GetTypeCode(type) != TypeCode.Object
            || type == typeof(Guid)
            || typeof(IImmutableParameter).IsAssignableFrom(type);
}

Edit : This approach (even if it offers developers the possibility to bypass the SetParameter in some optimization cases) is little bit hacky,the best way would be to had a "IsImmutable" property on System.Type...

I think this is a great idea, I don't think it's hacky. Most of the time higher components receive object models as parameters. It isn't convenient to render the entire tree. At least, in my opinion, optionally, we should be able to accept values as unchanged if reference equality (or value equality for records) is satisfied.

[alperenbelgic]

I haven't tested yet but I've come up with the below so far to address it. The actual component should call Initialize and if necessary HasValueChanged can be overridden, T can be a named tuple. What I am actually planning to use with Fluxor is the second section down below. But I am not so much aware of the possible consequences, I should play with it for a while.

public abstract class BaseComponent<T> : ComponentBase
{
    T oldValue;
    T currentValue;
    Func<T> ValueProvider;

    protected void Initialize(Func<T> valueProvider)
    {
        this.ValueProvider = valueProvider;
    }

    protected override bool ShouldRender()
    {
        return HasValueChanged(oldValue, currentValue);
    }

    protected virtual bool HasValueChanged(T oldValue, T currentValue)
    {
        return !oldValue.Equals(currentValue);
    }

    protected override void OnParametersSet()
    {
        UpdateValue(this.ValueProvider());
        base.OnParametersSet();
    }

    private void UpdateValue(T value)
    {
        oldValue = currentValue;
        currentValue = value;
    }
}

The second with Fluxor:

public abstract class BaseFluxorComponent<T> : FluxorComponent
{
    T oldValue;
    T currentValue;
    Func<T> ValueProvider;
    private IStateChangedNotifier[] StatesToRegister;

    protected void Initialize(Func<T> valueProvider, params IStateChangedNotifier[] statesToRegister)
    {
        this.ValueProvider = valueProvider;
        this.StatesToRegister = statesToRegister;

        foreach (var state in statesToRegister)
        {
            state.StateChanged += StateChanged;
        }
    }

    protected override bool ShouldRender()
    {
        return HasValueChanged(oldValue, currentValue);
    }

    private void StateChanged(object? sender, EventArgs e)
    {
        UpdateValue(this.ValueProvider());
    }

    protected virtual bool HasValueChanged(T oldValue, T currentValue)
    {
        return !oldValue.Equals(currentValue);
    }

    protected override void OnParametersSet()
    {
        UpdateValue(this.ValueProvider());
        base.OnParametersSet();
    }

    private void UpdateValue(T value)
    {
        oldValue = currentValue;
        currentValue = value;
    }

    protected override void Dispose(bool disposing)
    {
        foreach (var state in StatesToRegister)
        {
            state.StateChanged -= StateChanged;
        }

        base.Dispose(disposing);
    }
}

[ghost]

Hello. I see that you've just added Needs: Author Feedback label to this PR. That label is for Issues and not for PRs. Don't worry, I'm going to replace it with the correct one.

[ghost]

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime. To make sure no breaking changes are introduced, please remove the pending ci rerun label to kick off a new CI run.

[ghost]

Hi @alix-tlse. It seems you haven't touched this PR for the last two weeks. To avoid accumulating old PRs, we're marking it as stale. As a result, it will be closed if no further activity occurs within 4 days of this comment. You can learn more about our Issue Management Policies here.

[javiercn]

We had a bunch of discussion within the team, and we think this is mostly ready to go. The only thing we are missing is overriding Equals and GetHashCode on EventCallback and EventCallback<T> to avoid the default struct comparison and the corresponding benchmark to make a call on that change.

[alix-tlse]

Hello, my performance tests, still on my laptop with .NET 6 with a wasm application, gives for a million tests to Equals method :

• 3 242 ns without overriding methods (struct comparison)
• 650 ns with overriding of the GetHashCode and Equals methods . I updated my pull request.

It is more than 4 times faster to overload the Equals method.

[alix-tlse]

I try to fix the following error :

error RS0016: (NETCORE_ENGINEERING_TELEMETRY=Build 'GetHashCode'/'Equals' is not part of the declared API.

So I update the PublicAPI.Shipped.txt, but now I have the following error :

Detected modification to baseline API files. PublicAPI.Shipped.txt files should only be updated after a major release. See /docs/APIBaselines.md for more information.

I need help !

It's not an easy road to have a pull request accepted ;-)

[javiercn]

So I update the PublicAPI.Shipped.txt, but now I have the following error :

Did you update it by hand or did you use the analyzer to update the baseline. It should take care of it automatically.

It's not an easy road to have a pull request accepted ;-)

This one is tricky because it touches on a very low-level core piece of functionality, so we need to make sure we do not regress anything as the impact of doing so is big. Thanks for continuing to push for it, we are almost at the end of the line.

[alix-tlse]

Did you update it by hand or did you use the analyzer to update the baseline. It should take care of it automatically.

It's all good, I took the time to read the file /docs/APIBaselines.md and moved my lines from PublicAPI.Shipped.txt to PublicAPI.Unshipped.txt

[alix-tlse]

Hello @javiercn, will this pull-request be merged into a preview version of .NET 8?

[javiercn]

@alix-tlse thanks for the persevering and getting the contribution through!

[blake-fm]

Just wanted to acknowledge the importance of this PR (at least to us): I would much rather wait for .NET 8 RTM and eat some minor hardware costs, than refactor enum params out of one of our projects.

Thank you. Very much.

[ghost]

Hi @blake-fm. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.