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.55k stars 10.05k forks source link

Blazor Hybrid - Performance UpdateDisplayAsync #43867

Open softwaretirol opened 2 years ago

softwaretirol commented 2 years ago

Is there an existing issue for this?

Describe the bug

We have a component with a high frequency of updates in case the user is scrolling within the component. This feels a bit laggy, and we have looked up the situation with dottrace.

We optimized our implementation already, but we see an high CPU usage within the Internals of the WebView Component of Blazor. Especially the Method "UpdateDisplayAsync".

image

I can see here two things, one is the Serialize Method of the IpcCommon class, and also the PostWebMessageAsString Method of the WebView2 component.

Do you have any advise we can improve the situation here?

Expected Behavior

Fast Performance :-)

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version

6.0.8

Anything else?

Blazor Hybrid with WPF Integration.

<PackageReference Include="Microsoft.AspNetCore.Components.WebView.Wpf" Version="6.0.486" />
<PackageReference Include="Microsoft.AspNetCore.Components.Forms" Version="6.0.8" />
<PackageReference Include="Microsoft.AspNetCore.Components.Web" Version="6.0.8" />
javiercn commented 2 years ago

@softwaretirol thanks for contacting us.

There are limitations on what we can do here since unfortunately everything between the .NET side and the Webview2 needs to be marshalled as a string to the browser.

It is impossible for us to offer guidance without getting a clear picture of the scenario. Performance is a moving target and there are limits to what we can achieve imposed by the underlying components we build on.

My first recommendation would be to analyze two things:

For us to look in more detail we might need a minimal repro project that illustrates the problem (and steps to reproduce it) so that we can measure and determine if this is expected or there is an issue.

ghost commented 2 years ago

Hi @softwaretirol. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

softwaretirol commented 2 years ago

Hi,

i tried to recreate a small sample to identify the bottle neck. See here: https://github.com/softwaretirol/PerformanceBlazorWpf

It simulates a huge change every 100ms. image

As you can see we have the same situation here, that the Serialization of the IPC Command is taking a huge amount of time?...

javiercn commented 2 years ago

@softwaretirol thanks for the additional details.

I think this is expected. If your repro is representative of your app, you are doing way more work than what you need to. Like every system, rendering does not scale infinitely and unfortunately, due to the way WebViews work, the only way to communicate between .NET and C# is via the WebView interop mechanism that only accepts strings. That means we end up serializing all the data and converting it to a base64 string to marshall it back and forth. This is, AFAIK a limitation all webview based frameworks have.

That is not a problem per-se, just the reality webviews on different platforms impose on hosts (and there are likely reasons like memory safety), but it does have an impact that you need to account for when you build your app. In general, the sample you provided will not likely work well any UI framework.

The app in this case is simply doing too much work, so the first recommendation would be to reduce the amount of work you are doing. Specifically,

Finally, even if we were to magically cut the time in half, you would get a ~7.5% perf improvement, which I do not think would significantly change the experience.

Our suggestion would be that you analyze your app to understand what are the parts that are causing rendering to be expensive and try to apply some of the suggestions here

<div id="scroller">
    @for (int i = 0; i < 10000; i++)
    {
        <div>@DateTime.Now.AddSeconds(i).Ticks</div>
    }
</div>

@code 
{
    protected override void OnInitialized()
    {
        base.OnInitialized();
        Start();
    }

    private async void Start()
    {
        while (true)
        {
            await Task.Delay(100);
            await InvokeAsync(StateHasChanged);
        }
    }
softwaretirol commented 2 years ago

Thanks for your recommendations, i know the root cause and also the virtualization technique. Sadly the demo shows just the same problem but not our situation.

Just for reasons of interest: are there plans to communicate with the WebView2 engine in another way in the future? I see that there are room for improvements...

javiercn commented 2 years ago

Just for reasons of interest: are there plans to communicate with the WebView2 engine in another way in the future? I see that there are room for improvements...

Unfortunately, it is unlikely there will be major changes in this area as this requires the underlying webview platform components to offer better APIs to achieve interop with the host.

softwaretirol commented 2 years ago

One intresting fact: https://github.com/dotnet/aspnetcore/blob/de68feabb4dc84d2d5fbcfd7b3ed0b08f53216fa/src/Components/WebView/WebView/src/IpcSender.cs#L32

Currently the data is converted to Base64, and afterwards it is passed to the JsonSerializer. I wonder why that is done this way? If you adapt the code like the following:

image

The array is passed to serializer, and the serializer is handling the Base64 encoding. In this case the serialization is much faster (maybe because of the escape-check in the long string?).

image

javiercn commented 2 years ago

@softwaretirol Could you collect a performance trace of your app (run in Release without Debugging) where you are reproducing the performance problem and create an issue through the Report a problem with Visual Studio - Visual Studio (Windows) | Microsoft Docs dialog and attach the trace there (link this issue in that issue and link the VS issue here). It will help us get a better picture of the scenario and analyze other things that might be impacting performance.

javiercn commented 2 years ago

@softwaretirol I think there is an explanation here we did not have evidence to continue optimizing this code path. Passing the array directly might be something to consider in this care.

ghost commented 2 years ago

Thanks for contacting us.

We're moving this issue to the .NET 8 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.

gdar91 commented 1 year ago

@javiercn That's sad. Can we at least use a higher base (like Base128) to at least squeeze more performance out of it? Maybe also consider interoping via a network protocol between those two processes, where the host serves a local HTTP/WebSocket connection and WebView sits on it.

ghost commented 1 year 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.

ghost commented 1 year 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.