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

Performance issue: data table with pagination #6883

Closed
Ploppy3 opened this issue Sep 6, 2017 · 10 comments
Closed

Performance issue: data table with pagination #6883

Ploppy3 opened this issue Sep 6, 2017 · 10 comments
Assignees

Comments

@Ploppy3
Copy link

Ploppy3 commented Sep 6, 2017

Bug, feature request, or proposal:

Bug

What is the expected behavior?

Data Table should not recreate the rows when going to the next page.

What is the current behavior?

Data Table should reuse the rows when going to the next page.

What are the steps to reproduce?

https://material.angular.io/components/table/overview#pagination

What is the use-case or motivation for changing an existing behavior?

Angular Material has several performance issues which I think should be addressed.
The web is already slow, no need to make it any slower.
Consider using trackBy feature.

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

@angular/material: ^2.0.0-beta.10

@andrewseguin
Copy link
Contributor

Can you include some performance metrics that you used to see that this is slowing down the table?

@andrewseguin andrewseguin self-assigned this Sep 6, 2017
@Ploppy3
Copy link
Author

Ploppy3 commented Sep 6, 2017

The browser recreates the dom instead of reusing it.
This tutorial explains it better that I could: https://netbasal.com/angular-2-improve-performance-with-trackby-cc147b5104e5

@andrewseguin
Copy link
Contributor

That article describes how trackBy can be used within ngFor so that changes made to the data can be recorded to the IterableDiffer using a different algorithm than checking object references.

This is the same logic we use in the table. The table reuses practically the exact code and underlying IterableDiffer to check if we need to recreate row templates. In fact, the table does provide a trackBy input so that you can help the IterableDiffer understand if changes have been made.

However, it sounds like you want us to reuse the row template between two different sets of data. This is different than what ngFor does. If we do this, we need to some extra overhead:

  • IterableDiffer should let us iterate over all deletions first so we can capture any row templates that will not be used
  • Each addition should grab one of these row templates, clear its template, then insert its own cells in

Note that since IterableDiffer is in essence a linked list of changes, ordering it so that deletions are first will likely be an O(n log n) operation, which would be detrimental the table's performance. It possible we could find a middleground by only re-using a row if we find a deletion that precedes an addition so we can avoid the sort, but we won't be able to reuse all the templates. Also, we'll add cost when we clear out the row's cells in the template.

Finally, since we'll be adding in a when predicate to rows, which means there are multiple row templates, we'll need to make sure we are only reusing rows such that the we are correctly matching row templates according to the new data.

Given the cost of these things, we really need performance metrics that help us understand how the table is slow before we begin to make such changes. We are striving to make the table as performant as possible while providing a capable set of features. If you find an optimal set of logic that we can use, we are all ears, but we do really need some metrics that helps back up the changes.

@Ploppy3
Copy link
Author

Ploppy3 commented Sep 6, 2017

My knowledge is obviously weak but, to render a table with pagination & sorting, I would use one array to store all the rows and an other one to store the rows to render.
Sorting could be done on the first array, switching page would just copy the new requested rows to the rendering array.
Rendering could be done with ngFor on the rendering array and the trackBy function would be use to reuse the row template by using the indexes of this array rather than recreating them every time the page changes.

What are you thoughts on this?

@andrewseguin
Copy link
Contributor

What you're suggesting would add a lot of overhead and memory to the table, as well as couple the table to the concepts of sorting and pagination. This would likely harm the table's performance rather than improve it, and make it much less flexible for broad generic usage. You're welcome to clone the repo and test any changes you are proposing. If you find something that works more efficiently, feel free to send us a PR.

@Ploppy3
Copy link
Author

Ploppy3 commented Sep 8, 2017

After investigation on a data-table with pagination which I created last year, as long as you don't use complex components in your table, the render time are similar.

Though, if you added some performance heavy components to the rows such as an md-button, the md-table will be much slower than table which would reuse the DOM.

@SpEcHiDe
Copy link

Hi. Is it possible to create two different data tables with pagination in one Component?

I tried to read the documentation, but I did not find anything.

@andrewseguin
Copy link
Contributor

andrewseguin commented Oct 27, 2017

@SpEcHiDe Shouldn't be a problem, just use template reference strings to grab the paginator, e.g.

Template:

<mat-paginator #mySecondPaginator>

Component TS:

@ViewChild('mySecondPaginator') mySecondPaginator: MatPaginator;

Here's a plunker as an example of two tables with separate paginators: https://plnkr.co/edit/GfRj8bxsRjauoqxJil8C?p=preview

@cdskill
Copy link

cdskill commented Mar 5, 2018

Hi I'm litle bit stuck because I want to assign multiple paginator on multiple datatable generated...
For exemple the server send me 4 tables, each one with their pageSize and their pagination...

How can I perform that ?

I can actually generate multiple datatable but not appliying the pagination on each one.

This is how my datatable are generated (mat-paginator is not working):

				<div *ngIf="subcategory.completed && subcategory.total > 0">
					<mat-table #table [dataSource]="castMatatableDataSource(subcategory.content.rows)">
						<ng-container *ngFor="let column of subcategory.content.columns" [matColumnDef]="column.prop">
							<mat-header-cell *matHeaderCellDef>{{column.name}}</mat-header-cell>
							<mat-cell *matCellDef="let row" [innerHtml] ="row[column.prop]">{{row[column.prop]}}</mat-cell>
						</ng-container>

						<mat-header-row *matHeaderRowDef="subcategory.content.props()"></mat-header-row>
						<mat-row *matRowDef="let row; columns: subcategory.content.props()"></mat-row>
					</mat-table>
					<button (click)="show(table)">SHOW</button>
					<mat-paginator #paginator
								[pageSize]="subcategory.content.per_page"
								[pageSizeOptions]="[subcategory.content.per_page]">
					</mat-paginator>
				</div>

Thank you!

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants