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

Table, pFrozenColumn: Frozen columns are extremely slow and kick off many change detection cycles #14579

Closed
natebundy opened this issue Jan 17, 2024 · 9 comments
Assignees
Labels
LTS-PORTABLE PRO Support Issue was reported by PRO User Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Milestone

Comments

@natebundy
Copy link

natebundy commented Jan 17, 2024

Describe the bug

The pFrozenColumn directive is kicking off an extra change detection cycle for every table cell that contains the directive. For example, with a table with 90 rows that each have a td as part of a frozen column, 90 extra change detection cycles happen. For a table with 2 frozen columns with 90 rows, 180 extra change detection cycles happen.

(The directive also kicks off unnecessary cycles when using pFrozenColumn with a false frozen input for dynamic frozen columns since setTimeout is being called either way)

In addition to the high number of change detection cycles, the frozen column code in updateStickyPosition is blocking the main thread and causing layout thrashing, which is freezing the UI for several seconds (and for one of our power users with a huge amount of data, over a minute!) with decently sized tables. This happens when we initially draw the table, when clearing a search, or doing any operation that causes frozen columns to be redrawn. For virtual scrolling up in large tables with a frozen column with all of the data loaded (especially when showing an additional frozen column, such as checkboxes for multiple selection), this causes several seconds of the browser thrashing with a blank table before finally being able to draw the table again. Setting the styles this way is potentially causing the browser to reperform layout for every single cell in a frozen column every time that table cell is initialized.

Environment

Slowness with table began when upgrading PrimeNG from 14 to 16. Nothing special about our environment.

Reproducer

https://stackblitz.com/edit/github-ddsd3p

Angular version

17.0.8

PrimeNG version

17.3.1

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

18.18.2

Browser(s)

No response

Steps to reproduce the behavior

Using the provided StackBlitz:

  1. Notice change detection cycles caused by loading a table with frozen columns

  2. Clicking "redraw table" will start change detection count at 0 and reload the table data. Notice how many cycles it takes to reload the table

  3. Search for something (e.g. "James") then notice how many change detection cycles happen when clearing out the search

  4. Click toggle table type and switch to the table without frozen columns, then repeat steps 1-3 and notice how many fewer change detection cycles happen

  5. Try searching while the change detection cycles are counting up on the table with frozen columns and notice that the search cannot respond until the change detection cycles are finished

  6. Notice how choppy scrolling is while the change detection cycles are firing

Expected behavior

Ideally, frozen columns should be implemented in a way that kicks off no more than 1 extra change detection cycle per column (or even 0 or 1 total extra cycles), no extra cycles for dynamically frozen columns with a false input, and should only set styles once per column instead of once per cell. (Frozen columns will still have to be recalculated where column sizes may change - e.g. when virtual scroll brings in new items or when filtering table data - since automatic table sizing may change column sizes in such cases, but those should follow the same rules. Perhaps a ResizeObserver could handle such cases?) After upgrading from PrimeNG 14 to 16 and then 17, we've had to implement our own frozen columns in the meantime because the table performance with frozen columns has gotten so much worse.

@natebundy natebundy added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Jan 17, 2024
@cetincakiroglu cetincakiroglu added PRO Support Issue was reported by PRO User Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Feb 29, 2024
@cetincakiroglu cetincakiroglu self-assigned this Feb 29, 2024
@cetincakiroglu cetincakiroglu added this to the 17.10.0 milestone Feb 29, 2024
@cetincakiroglu cetincakiroglu added LTS-PORTABLE and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Mar 4, 2024
@cetincakiroglu
Copy link
Contributor

Hi,

Thanks for reporting the issue, could you please try and give us feedback with the v17.10.0 after this week's release?

@cetincakiroglu cetincakiroglu added the Type: Bug Issue contains a bug related to a specific component. Something about the component is not working label Mar 4, 2024
@natebundy
Copy link
Author

At first glance, while this should indeed clean up the extra change detection cycles, updateStickyPosition firing on every frozen table cell is more than likely still blocking the main thread and causing greatly delayed user interactions. I'll confirm and put up another StackBlitz that demonstrates that if it's still happening.

@cetincakiroglu
Copy link
Contributor

@natebundy

Thanks for the feedback, I've tested with your example in my local. Running outside of the zone removed unnecessary change detection calls. Could you please test it after the release and tag me if anything is wrong?

@intracloud
Copy link

me too.
i have p-table with 7 frozen columns and 400 rows.
It's extremely slow, even Chrome shows message to close the window
Thanks.

@AgusEDSA
Copy link

We have the same issue, a table with 12 columns, 2 frozen, on 500+ rows it starts freezing the browser.

@natebundy
Copy link
Author

@intracloud @AgusEDSA when you both get a chance, try 17.10. It does seem like a marked improvement for frozen column performance for my app. Initially rendering large tables with frozen columns can still take a second or two, but once they're rendered they're much more responsive. However, we're usually maxing out at 2-3 frozen columns, so it'll be interesting to hear if this is still the case with 7.

If you immediately click and hold the down arrow on the vertical scrollbar after the table with frozen columns loads (or after pressing "Redraw table"), you can see a huge difference between these two:
17.3.2
17.10.0

17.3.2 will literally freeze to a halt on my machine while 17.10.0 keeps scrolling along.

@intracloud
Copy link

hi @natebundy

The performance problem isn't when scrolling up to the bottom or right left.
The problem is that when the p-table loads the data, without pFrozenColumn it takes a few seconds, with pFrozenColumn 10 or 20 times longer.

This problem is the same when I have 3 or 4 pFrozenColumn columns, I have so many because there are columns that have p-buttons to execute actions on the record.

This performance problem does not exist in version 14.1.2, the data loading is fast, but the problem is that the pFrozenColumn columns do not work correctly with p-buttons or icons

Thanks.

@Xapuu
Copy link

Xapuu commented Apr 15, 2024

After fighting the same issue with versions 16+ and 17.13, ended up with writing my own CSS instead of using the frozen mechanism.
StackBlitz

It works for me, hopefully it will help someone else that ends up in this thread.

@cuongbrilliantSE
Copy link

@Xapuu It works when there's only one frozen column."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LTS-PORTABLE PRO Support Issue was reported by PRO User Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Projects
None yet
Development

No branches or pull requests

6 participants