DateOnly and TimeOnly is not supported in Blazor change detection for parameters

[Andrzej-W]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

DateOnly and TimeOnly is probably supported almost everywhere. Now it is time to add them to known immutable types here (similar to Guid):

aspnetcore/src/Components/Components/src/ChangeDetection.cs

Lines 42 to 45 in 213eb28

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

GetTypeCode returns TypeCode.Object for both of them.

Expected Behavior

No response

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version

No response

Anything else?

No response

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

[SteveSandersonMS]

Thanks for the suggestion. As indicated in the comment above that line of code, this is a careful perf tradeoff. It might not be intuitively obvious, but this is at such a perf-critical point in rendering that adding more "if" clauses here can worsen perf in perceptible ways (at least on WebAssembly). In general, this check is not intended to know about all possible immutable types; it's only intended to be a quick first approximation where if it returns true then it's definitely immutable, and if it returns false then we don't know whether it's immutable (so it's always legal to return false).

The current version of this check was a very carefully balanced choice rooted in a lot of benchmarking: #42901

If you'd be interested in recreating some of those benchmarks to compare the impacts of adding checks for DateOnly and TimeOnly, that could definitely help us move this suggestion forwards.

[Andrzej-W]

I have made a few performance tests on different hardware in Blazor WASM and Blazor WASM with AOT compilation. Program was compiled in VS 2022 17.6.4, .NET SDK 7.0.304. Here are the original and modified version of IsKnownImmutableType function:

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

private static bool IsKnownImmutableTypeDTOnly(Type type)
    => Type.GetTypeCode(type) != TypeCode.Object
    || type == typeof(Guid)
    || type == typeof(DateOnly)
    || type == typeof(TimeOnly)
    || typeof(IEventCallback).IsAssignableFrom(type);

Call to IsAssignableFrom in the new version is intentionally at the end to show interesting benchmarks.

I have made tests with the following types:

• int the fastest path
• object the slowest path
• DateOnly and TimeOnly to show two new types
• EventCallback - time almost identical to Object (not presented in the table below)

Hardware/software used (all updates installed):

• MSI laptop with Core i7-12700H, Windows 11 Pro 22H2, Edge 114
• Apple MacBook Air with M1 processor, MacOS, Safari
• iPhone 12 Pro Max, iOS 16.5.1, Safari
• iPhone 6s, iOS 15.7.7, Safari
• budget Xiaomi Redmi 9A, Android 10, Chrome 114 (it is so slow that it is almost useless)

Time in ms for 1_000_000 calls.

Hardware	Software	Old int	Old Object	New Object	New DateOnly	New TimeOnly
MSI	WASM	45	356	390	70	86
MacBook	WASM	51	340	392	75	97
iPhone 12	WASM	70	463	537	115	146
iPhone 6s	WASM	180	1400	1566	295	369
Redmi 9A	WASM	800	6847	7539	1339	1686
MSI	AOT	6	66	75	14	18
MacBook	AOT	3	60	68	9	13
iPhone 12	AOT	6	73	85	18	24
iPhone 6s	AOT	20	249	233	51	68
Redmi 9A	AOT	132	1078	1215	267	340

This table contains three phenomena. MacBook in WASM interpreted mode is slower than MSI but old function with Object type is faster. MacBook in WASM AOT mode is always faster than MSI. On iPhone 6s in WASM AOT mode new function with Object type is faster than old one. I repeated this at least 10 times with 100_000, 1_000_000, 10_000_000 repetitions and new function was always faster!!!

In the worst case when function evaluates all conditions it is 10-16% slower. On the other hand for DateOnly and TimeOnly it will be several times faster than the old one and as a bonus there is a chance that component will not be rendered.

[SteveSandersonMS]

Thanks very much for investigating this so deeply. Your numbers gave me a further idea about how this could be optimized more while also having more flexibility to include other types in the future without significant perf regressions.

We've been resisting having any kind of dictionary/hashset lookup here due to benchmarking showing that the key hashing and dictionary traversal taking too long, plus the fact that we need to track EventCallback<T> for arbitrary T means we have to allow the lookup to expand dynamically at runtime which limits its perf options further.

However, it is possible to combine multiple optimizations in a way that uses a dictionary/hashset for one particular code path which seems to give better numbers overall. The function I just tried looks like:

    // This is my new candidate (other code below is just to support it)
    private static bool IsImmutable_HybridConcurrentDict(Type type)
        => Type.GetTypeCode(type) != TypeCode.Object
        || _immutableNonObjectTypesCache.GetOrAdd(type, IsImmutableNonObjectCore);

    private static ConcurrentDictionary<Type, bool> _immutableNonObjectTypesCache = new();

    private static bool IsImmutableNonObjectCore(Type type)
        => type == typeof(Guid)
        || type == typeof(TimeOnly)
        || type == typeof(DateOnly)
        || typeof(IEventCallback).IsAssignableFrom(type);

Results:

• For int, string (or anything satisfying Type.GetTypeCode(type) != TypeCode.Object), it's indistinguishable from the existing logic as it only runs the same checks
• For object or other custom types, it's quite a bit faster than the existing logic, and faster still than if we also check for TimeOnly/DateOnly
• For Guid, it is slower than the existing logic
• For TimeOnly/DateOnly, it's faster than the existing logic (and returns true), but slower than the proposed update that checks explicitly for TimeOnly/DateOnly

Overall this seems like a potentially preferable tradeoff as it improves all existing app performance (except guids, but that's very niche), adds support for TimeOnly/DateOnly (albeit not maximally optimized for those), and gives us an easy way to add further types in the future in such a way that we shouldn't see the perf meaningfully degrade for each added type. What do you think?

It might be possible to go a bit faster still by using Dictionary<T, U> instead of ConcurrentDictionary<T, U> but then we would have to do some manual extra locking. For example it could have a sort of double-buffered system where there are two dictionaries, only one of which is read from, and then if an update is needed the thread would take a lock on the second one, build it, and then atomically swap over the handle to the current dictionary used for reading. I'm not really inclined to add that sort of sophistication unless it's massively improving the numbers.

Caveat The actual numbers I've recorded on my machine are ~2x slower than you seem to get. I'm testing on the WebAssembly interpreter, also from the .NET 7 SDK, and in current Edge browser. I don't yet understand why my numbers are 2x slower than yours, since I'm running an i9 that should be faster. So, to understand this a bit more, would you be willing to try out the proposed version above to see how it compares with your candidate on your hardware, just in case there's something strange going on with mine?

[Andrzej-W]

Hello Steve

The actual numbers I've recorded on my machine are ~2x slower than you seem to get.

Although I don't play games, my laptop is a big, heavy gaming laptop with efficient cooling system. It is configured in performance mode. I also turned on the turbo mode, in which the fans run continuously at maximum efficiency. The laptop was connected to the power supply. Maybe your laptop is smaller, lighter business model configured for balanced performance.

I will check your solution on my hardware/software in a few minutes.

[danmoseley]

Is the dictionary immutable here (sounds like not)? If it is then the new FrozenDictionary might be worth measuring.

[SteveSandersonMS]

Unfortunately it's not immutable because we need EventCallback<T> (for arbitrary T that we don't know in advance) to resolve as true.

[Andrzej-W]

It looks that today my laptop is a little slower (I don't know why). The slowest path (Object type) for original version on my laptop (WASM interpreted) is today 400ms, my version of IsKnownImmutableType takes 454ms, hybrid version 300ms. It looks that hybrid version is really good and dictionary lookup is faster than IsAssignableFrom.

I like hybrid version because we can add new types almost for free.

NOTE: I removed question about nullable types - they are already handled in MayHaveChanged method.

[Andrzej-W]

I repeated all tests with hybrid version on a few devices in WASM and WASM AOT mode. Hybrid version is always faster than original version with Object type (slowest path). Usually it is more than 15% faster. Old iPhone 6s is a special case where hybrid version in WASM AOT mode is only marginally faster (less than 5%).

[SteveSandersonMS]

Thanks @Andrzej-W for looking into this.

Based on this analysis, I think we would be happy to go ahead with this new hybrid version. However, the team is fully occupied with committed work for .NET 8, so whether we do so likely comes down to whether you'd be happy to make a PR for it. The PR would need to:

• Update the logic itself
• Update the comment around it
• Update the test SkipsUpdatingParametersOnChildComponentsIfAllAreDefinitelyImmutableAndUnchanged to account for the new behaviors for DateOnly/TimeOnly
• Since it's introducing a new cache, hook into the hot reload notification so it clears the cache when a hot reload event occurs. It's only about 5 lines of code - as an example see Router.cs and how it uses MetadataUpdateSupported and OnDeltaApplied.

Is that something you'd be interested in doing? No problem if not!

[Andrzej-W]

Hello @SteveSandersonMS
I haven't contributed to ASP.NET repository yet. I will read your doc and try to make a pull request. Give me a few days because I'm too busy right now.

What do you think about adding two more types:

• TimeSpan (https://learn.microsoft.com/en-us/dotnet/api/system.timespan?view=net-8.0)
• DateTimeOffset (https://learn.microsoft.com/en-us/dotnet/api/system.datetimeoffset?view=net-8.0).

As an example DateTimeOffset can be used in DxDateEdit component (https://docs.devexpress.com/Blazor/DevExpress.Blazor.DxDateEdit-1) and TimeSpan can be used in DxTimeEdit (https://docs.devexpress.com/Blazor/DevExpress.Blazor.DxTimeEdit-1) and MudTimePicker components (https://mudblazor.com/components/timepicker).