dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.37k stars 9.99k forks source link

EventCallback closures lead to increased diff size and decreased Blazor performance. #31755

Closed jspuij closed 9 months ago

jspuij commented 3 years ago

Describe the bug

When using lambdas that capture variables as event callbacks in Blazor components, the diffs will contain new event handler ids whenever the component is rerendered. Especially in components containing loops this will lead to increasing diff sizes and decreased performance. A detailed explanation and discussion of a fix can be found on my blog.

https://spuij.nl/closures-as-callbacks-improving-blazor-performance-part-1/ https://spuij.nl/closures-as-callbacks-improving-blazor-performance-part-2/ https://spuij.nl/closures-as-callbacks-improving-blazor-performance-part-3/

This bug was previously reported as https://github.com/dotnet/aspnetcore/issues/17886, but was closed as a workaround is possible for the event callback situation. However the issue extends to Two-way-binding as well, and the closure is created by the Razor compiler here and is harder to be avoided. Since I've already devised a fix, I'll attach a PR to this issue.

To Reproduce

Clone https://github.com/jspuij/DelegateBindingTest. Run the project. Open the developer tools of Edge. Inspect the web socket connection and the messages. Notice the increasing diff size whenever you add a row to the table. This is true for the event callback page, and the binding page.

Exceptions (if any)

No exceptions.

Further technical details

6.0.0-preview.4.21211.7

.NET SDK (reflecting any global.json): Version: 6.0.100-preview.3.21168.19 Commit: 82e763ce49

Runtime Environment: OS Name: Windows OS Version: 10.0.19042 OS Platform: Windows RID: win10-x64 Base Path: C:\source\external\AspNetCore.dotnet\sdk\6.0.100-preview.3.21168.19\

Host (useful for support): Version: 6.0.0-preview.4.21211.7 Commit: 355eff52be

.NET SDKs installed: 5.0.100-rc.1.20379.10 [C:\source\external\AspNetCore.dotnet\sdk] 6.0.100-alpha.1.20523.3 [C:\source\external\AspNetCore.dotnet\sdk] 6.0.100-preview.3.21168.19 [C:\source\external\AspNetCore.dotnet\sdk]

.NET runtimes installed: Microsoft.AspNetCore.App 3.1.4 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.13 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 5.0.0-dev [C:\source\external\AspNetCore.dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 5.0.0-rc.1.20379.2 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 6.0.0-alpha.1.20513.8 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 6.0.0-dev [C:\source\external\AspNetCore.dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 6.0.0-preview.3.21168.5 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 2.1.18 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.25 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.4 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.13 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 5.0.0-rc.1.20371.13 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.0-alpha.1.20428.2 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.0-alpha.1.20513.9 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.0-alpha.1.20560.10 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.0-preview.3.21167.1 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.0-preview.4.21201.1 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.0-preview.4.21211.1 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.0-preview.4.21211.7 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.NETCore.App] Microsoft.WindowsDesktop.App 5.0.0-rc.1.20374.2 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 5.0.0-rc.1.20417.4 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 6.0.0-preview.3.21167.3 [C:\source\external\AspNetCore.dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET runtimes or SDKs: https://aka.ms/dotnet-download

Microsoft Visual Studio Enterprise 2019 Version 16.9.3

ghost commented 3 years ago

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

ghost commented 3 years ago

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.

ghost commented 9 months ago

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

mkArtakMSFT commented 9 months ago

We did spend quite a bit of time discussing how this could be achieved but we couldn't come up with a reasonable solution (a good balance of simplicity and benefit) here. Hence, closing this for now. We may revisit this in the future, when we come up with some good way to support this.