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

Nested rows `collapseAll()` and `expandAll()` very slow on large data sets #5711

Closed
aaronbeall opened this issue Jan 10, 2019 · 2 comments

Comments

Projects
None yet
4 participants
@aaronbeall
Copy link
Contributor

commented Jan 10, 2019

Description

With larger data sets the collapseAll() and expandAll() APIs freeze the UI and take awhile to complete. In my real app I've seen it take as long as 30 seconds. I ran into this with nestedRows but I also tried using trimRows directly and had similar issues. Profiler shows a lot of time spent in arrayEach and collapseMultipleChildren as well as repeated GC activity.

Did a little profiling and I'm not totally clear, but it seems to be spending a lot of the time in this function: https://github.com/handsontable/handsontable-pro/blob/af54e7ea124d594fef9058e2b7ca0b2160fd3a39/src/plugins/nestedRows/ui/collapsing.js#L111

The (anonymous) function below is the callback in the arrayEach linked above:

image

image

I am not sure but since rowsToTrim = rowsToTrim.concat(this.collapseChildren()) is creating a new array with every iteration, possibly that is causing memory allocation churn and GC which slows things down? Maybe rowsToTrim.push(...this.collapseChildren()) would improve things?

Steps to reproduce

  1. Open https://jsfiddle.net/k74xfL5d/ which is table with 5000 rows and 20 nested rows for each row.
  2. Click "Collapse All", which calls getPlugin("nestedRows").collapsingUI.collapseAll()
  3. Note the UI freezes for a number of seconds

Demo

https://jsfiddle.net/k74xfL5d/

Your environment

  • Handsontable version: 6.2.2
  • Browser Name and version: Chrome 71.0.3578.98
  • Operating System: macOS Mojave 10.14.1
@aaronbeall

This comment has been minimized.

Copy link
Contributor Author

commented Jan 11, 2019

I hacked at the code and found that I got a huge speed improvement by changing the implementation of collapseMultipleChildren() and expandMultipleChildren() where it used concat() to

    arrayEach(rows, (elem) => {
      rowsToTrim.push(...this.collapseChildren(elem, false, false));
    });

and

    arrayEach(rows, (elem) => {
      rowsToUntrim.push(...this.expandChildren(elem, false, false));
    });

respectively, along with changing untrimRows() implementation where it was using arrayEach() and splice() to

this.trimmedRows = this.trimmedRows.filter(r => rows.indexOf(r) == -1)

I'll put together a PR for this...

@AMBudnik

This comment has been minimized.

Copy link
Contributor

commented Jun 12, 2019

Fixed! :) Thank you for contributing.

@AMBudnik AMBudnik closed this Jun 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.