Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential Performance Issue #16044

Closed
neerajbharti opened this issue May 9, 2018 · 8 comments
Closed

Potential Performance Issue #16044

neerajbharti opened this issue May 9, 2018 · 8 comments
Labels
area-blazor Includes: Blazor, Razor Components

Comments

@neerajbharti
Copy link

Created a blazor project from default template. Then modifed the index.cshtml as the below code. View the output in browser. Click on any table row to hightlight it, it does work, but the highlighting happens after a delay. Is this a performance issue or am I doing something wrong? Please guide me to correct way of doing similar thing if my approach is wrong.
image

@page "/"
<style>
    .highlighed-record {
        background: red !important;
    }
</style>
<button onclick="@(()=> { this.PopulateList(); })">Populate List</button>
<br />
<label>Click on the table row below to mark it highlighted, and notice the sluggishness</label>
<table class="table table-bordered table-condensed">
    @foreach (Person p in this.lstPersons)
    {
        <tr onclick="@(()=>currentPerson=p)" class="@(p==currentPerson?"highlighed-record":"")">
            <td>@p.FirstName</td>
            <td>@p.LastName</td>
        </tr>
    }
</table>
@functions{
    List<Person> lstPersons = new List<Person>();
    Person currentPerson;
    class Person {
        public string FirstName;
        public string LastName;
    }
    void PopulateList()
    {
        //create dummy objects of Person class and add them to a the list.
        for (int i = 0; i < 300; i++)
        {
            this.lstPersons.Add(new Person() { FirstName = "First Name " + i, LastName = "Last Name " + i });
        }
        Console.WriteLine("Added dummy records");
        this.StateHasChanged();
    }
}



@medokin
Copy link

medokin commented May 9, 2018

I had a similar performance issue with JS and bindings.
IMHO you should not change the value that is bound 300 times. Every row will recalculate and probably rerender which takes time.

Another approach is to add a bool prop to your person, like active and change this. Highlight only if active. After the change iterate the others and deactivate if active. This way max 2 rows will be rerendered. 🔢

No time for benchmark now, but if you do, drop a line 👍

@neerajbharti
Copy link
Author

neerajbharti commented May 9, 2018 via email

@neerajbharti
Copy link
Author

I modified the code to have property, but no luck.
However i feel if the component has few hundred bindings on it. Then the whole component (or entire app) starts behaving slowly.
To reproduce this problem, simply follow these steps:

  1. Create a new Blazor project with default template
  2. In index.cshtml, put the below code.
  3. Run the app
  4. Now when the app shows up in browser, click the checkbox and see the flag value written in the span quickly syncs with the value of the bound property
  5. Now click on the button to create 600 dummy bindings (there are 300 rows of table with 2 columns in each row)
  6. Now trying clicking the checkbox and notice the significant delay after which the span syncs with the bound property.
@page "/"
<style>
    .highlighed-record {
        background: red !important;
    }
</style>

<input type="checkbox" bind="@flag" />
<span>FLAG VALUE IS: @flag</span>
<br />
<button onclick="@(()=> { this.PopulateList(); })">Populate List</button>
<br />
<table class="table table-bordered table-condensed">
    @foreach (Person p in this.lstPersons)
    {
        <tr onclick="@(()=>currentPerson=p)" >
            <td>@p.FirstName</td>
            <td>@p.LastName</td>
        </tr>
    }
</table>
@functions{
    List<Person> lstPersons = new List<Person>();
    Person currentPerson;
    bool flag;
    class Person {
        public string FirstName;
        public string LastName;
    }
    void PopulateList()
    {
        //create dummy objects of Person class and add them to a the list.
        for (int i = 0; i < 300; i++)
        {
            this.lstPersons.Add(new Person() { FirstName = "First Name " + i, LastName = "Last Name " + i });
        }
        Console.WriteLine("Added dummy records");
        this.StateHasChanged();
    }
}

@neerajbharti
Copy link
Author

I guess having high number of bindings is a usual scenario especially when you have tables on your screen.

@dlr1
Copy link

dlr1 commented May 10, 2018

I have to say this is significantly slower when compared to an equivalent sample in angular, but as of now there are no optimizations whatsoever. so the performance I assume will improve by the time if/when they release.

Depending on how the diff of the DOM is done, performance would be good if there are minimal changes, so breaking down the component (if possible) might help.

@Andrzej-W
Copy link

I have just tested your second example with checkbox. To see any delay on my computer (Core i7 6700K, 32GB RAM, Win 10 1709 x64) I have clicked "Populate list" button 5 times. On Firefox delay is hardly noticeable. On Edge I see delay but it is acceptable. Chrome is horribly slow.

@SteveSandersonMS
Copy link
Member

Thanks for the clear repro information, @neerajbharti.

This particular scenario of using capturing-lambdas inside a long list has always been a perf hotspot for Blazor's rendering approach. We're aware of it and hope to improve it significantly, but right now it's not something there's any easy workaround for. When an upcoming version of C# supports caching methodgroup-to-delegate conversions, the CPU cost of this pattern will be vastly reduced.

If you really want to solve this in the short term for arbitrarily long lists, you'd have to look at a UI virtialization technique. I don't have a sample to post to you right now but I'm sure this is something we'll get to relatively soon. Then it's possible to have lists with billions of entries with no extra cost versus just one screen's worth of them.

We're tracking the general CPU perf question elsewhere so I'll close this.

Also I'm aware that Chrome is lagging a long way behind other browsers on WebAssembly perf currently. However we spoke with a V8 PM and dev lead, who have work underway to modernise and improve it drastically. So I'd expect Chrome to catch up with its competitors in this area while Blazor is still in its experimental prerelease phase.

IMHO you should not change the value that is bound 300 times. Every row will recalculate and probably rerender which takes time.

Don't worry about making multiple changes. The render cycle will only start once you yield the thread (so only once, after all the 300 mutations).

@neerajbharti
Copy link
Author

neerajbharti commented May 12, 2018 via email

@mkArtakMSFT mkArtakMSFT transferred this issue from dotnet/blazor Oct 27, 2019
@mkArtakMSFT mkArtakMSFT added the area-blazor Includes: Blazor, Razor Components label Oct 27, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Development

No branches or pull requests

6 participants