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.43k stars 10.02k forks source link

Blazor - Give More Control Over Render Fragment Diff Calculations #28833

Open legistek opened 3 years ago

legistek commented 3 years ago

Is your feature request related to a problem? Please describe.

The underlying problem is rendering performance.

The specific problem is that if a complex component needs to render frequently, I have no way to specify which parts of the render tree are clean or dirty other than to sub-divide into more components. BuildRenderTree is an all or nothing endeavor. If I selectively skip non-dirty elements, they wind up getting removed from the DOM. If the stuff I want to skip winds up being expensive, I'm out of luck.

The diff mechanism is great for preventing unneded DOM updates, but what about preventing unneeded diff evaluations in the first place? This is worst when it comes to string comparisons. We know strings are one of the biggest bottlenecks in interpreted WASM. Minimizing string comparisons I've found is one of the best way to squeeze more performance out of Blazor.

Describe the solution you'd like

I think there's a very simple solution: provide a way to tell RenderTreeBuilder to ignore a range of sequence numbers that were there in a previous render and should not be removed but just be left alone.

Since I use code for most of my rendering at the component library level, I wouldn't personally need a Razor-way of doing this. But you could probably create a special tag like @ignoreif([expression]) that would have the same effect; after the first render, if expression were true, anything in the @ignoreif block would just get completely left alone during diff/render.

What it comes down to is, sometimes I know better than Blazor when stuff can be left alone.

Additional context

Just some code-based examples of what I'm talking about:

Suppose I have a simple input control with a label on top:

private bool _labelHasChanged;
private string _label;
[Parameter]
public string Label 
{
   get => _label;
   set 
   {
      _label = value;
      _labelHasChanged = true;
   }
}

protected override void BuildRenderTree(RenderTreeBuilder builder)
{
   builder.OpenElement(0, "div");
   builder.AddContent(10, this.Label);
   builder.CloseElement();

   builder.OpenElement(30, "input");
   // other stuff
   builder.CloseElement();
}

So here, I know when Label has actually changed. What I'd love to be able to do is say:

protected override void BuildRenderTree(RenderTreeBuilder builder)
{
   if (_labelHasChanged || !_hasRendered) 
   {
      builder.OpenElement(0, "div");
      builder.AddContent(10, this.Label);
      builder.CloseElement();
   }
   else
   {
      builder.Ignore(0, 10);    // tells the RenderTreeBuilder to ignore all sequence numbers between 0 and 10 when diffing
   }

   builder.OpenElement(20, "input");
   // other stuff
   builder.CloseElement();
}

By doing this I would not only skip execution of 3 lines of code, but more significantly I would prevent Blazor from having to compare the current value of Label with what was rendered previously.

This is obviously a highly simplistic example. The real life application here is that I'm working on a DataGrid using my own derivative of the built in Virtualize component and trying to squeeze every last bit of performance out of it that I can. I know which items are going in and out of view each time there's a render. I want to avoid doing anything to the items that aren't new.

Each item is a DataGridRow. The DataGridRow instances for the items who were there on the last render are not dirty. Their individual BuildRenderTree methods won't get called because I override ShouldRender. But I want to take it a step further. I don't even want to go through the 15 or so lines of code it takes to set up a DataGridRow's relevant attributes inside the main for loop when I know none of them have changed.

Note I am using SetKey, and that's why the individual DataGridRow' instances get recycled during scrolling. That works well, but it's not enough. It's still cycling through every visible row (whether it was there previously or not), setting all its attributes to the same values every time there's a render. If setting those attributes is at all expensive (and I'm trying to make an MVVM framework with databinding, so sometimes it can be), I'm in trouble.

So what I'd like to be able to do in my main for loop that renders the visible rows is this:

builder.OpenRegion(0);
for (int i = startVisible; i <= endVisible; i++)
{
   int seq = i * 50; // or whatever
   if (i >= _lastStartVisible && i <= _lastEndVisible)
   {      
      // It was visible on the last render, so leave it completely alone
      builder.Ignore(seq, seq+49);
   }
   else
   {
      builder.OpenComponent(seq, typeof(DataGridRow));
              // a lot of other stuff that might be expensive to do unnecessarily
      builder.CloseComponent();
   }
}
_lastStartVisible = startVisible;
_lastEndVisible = endVisible;
builder.CloseRegion();

What else I've tried

So I thought this might be somewhat related to this closed issue: https://github.com/dotnet/aspnetcore/issues/16057, but the solution there mentioned by @SteveSandersonMS was to factor out into individual components. Again, though, while this prevents the individual items' BuildRenderTree's from getting called, you can't avoid unnecessarily reinitializing every row with its input parameters in the main for loop.

Another reason I think this would improve performance is that it actually avoids the need to use SetKey to preserve the relationship between the Component instance and the data item. I'm guessing that when you use SetKey, Blazor is looking in some dictionary to try to find an existing Component instance to potentially recycle. That is not going to be a cost-free operation especially with large item sets. If you gave me control over the diffing, though, we wouldn't have to worry about Blazor not knowing whether to make a new Component or not because it would just skip the render fragments that I tell it to skip.

(I will separately ask you to give me a way to control the lifecycle and re-use of Component instances, but one thing at a time ;))

Hope this all makes sense, and is feasible.

Many thanks.

mkArtakMSFT commented 3 years ago

Thanks for contacting us, @legistek. Also, it's great to see that you've put a lot of thought into it. Most of our team members are out for the rest of the year so there will be some delay in responding to you. But we will get back to our normal schedule from early January. Just wanted to set expectations.

Happy Holidays!

ghost commented 3 years ago

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. 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 11 months 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.