React-virtualized: FlexTable component becomes very slow with +10 columns

[gajus]

I have been working the past two days to replace our current grid implementation using https://github.com/developerdizzle/react-virtual-list with FlexTable provided by react-virtualized.

Here is a striped-down version:

<div style={{ flex: '1 1 auto' }} className={cn}>
    <AutoSizer>
        {({height, width}) => {
            return <FlexTable
                headerHeight={itemHeight}
                height={height}
                rowCount={data.length}
                rowGetter={({index}) => { return data[index]; }}
                rowHeight={itemHeight}
                width={width}
            >
                <FlexColumn
                    dataKey='bucketSize'
                    flexGrow={1}
                    label='bucketSize'
                    width={100}
                />
                <FlexColumn
                    dataKey='sym'
                    flexGrow={1}
                    label='sym'
                    width={100}
                />
                <FlexColumn
                    dataKey='time'
                    flexGrow={1}
                    label='time'
                    width={100}
                />
                <FlexColumn
                    dataKey='ask'
                    flexGrow={1}
                    label='ask'
                    width={100}
                />
                <FlexColumn
                    dataKey='asize'
                    flexGrow={1}
                    label='asize'
                    width={100}
                />
                <FlexColumn
                    dataKey='bid'
                    flexGrow={1}
                    label='bid'
                    width={100}
                />
                <FlexColumn
                    dataKey='bsize'
                    flexGrow={1}
                    label='bsize'
                    width={100}
                />
            </FlexTable>
        }}
    </AutoSizer>
</div>

Nothing out of ordinary.

However, when you scroll the table, there is a considerable lag/ jitter. Upon further inspection, Grid component is spending loads of time for each scroll event.

It looks like with addition of every column table scroll performance drops in half. With 22 columns, the table scroll becomes virtually unusable.

In comparison, our simple react-virtual-list implementation scales to hundreds of columns without any noticeable performance impact.

I have attached a CPU profile log.

CPU-20160615T154350.zip

[bvaughn]

FYI I've renamed your issue description. The Grid component absolutely does not become slow with 10+ columns as the demo easily shows 1,000,000 rows x 1,000,000 columns. You seem to be referring to the FlexTable becoming slow with 10 columns- which is something I have no experience with yet. I think at most I've used it with 5-6.

[bvaughn]

react-virtualized is a nights-and-weekends project for me so I can't look into this right away. But I'll try to make some time tonight after work to profile FlexTable a little to see where the slowness is coming from.

[bvaughn]

It's also worth stressing that- (particularly with React 15)- dev-mode performance is _not_ great. (This is not something specific to react-virtualized.)

I mention this because a production test app of a FlexTable with 20 columns is much better than a dev-mode test of the same:
http://rawgit.com/bvaughn/react-virtualized/master/playground/table.html

I wouldn't call it "_virtually unusable_", particularly in Chrome. But there's room for improvement.

[gajus]

I wouldn't call it "virtually unusable", particularly in Chrome. But there's room for improvement.

I am pretty sure that depends on the computer. As with many developers, you assume that everyone has a comparable computer. The reality is that an avg. developer has computer that's way above the mean.

In the office setup, I had to use Chrome task manager to kill the tab after trying 20 columns.

However, you are right in saying that production build performance is a lot better.

[bvaughn]

Thanks but I'm aware of that. I test on environments other than my Macbook. My cellphone is a couple of generations old and my Virtual Box VMs are also pretty resource-constrained (4gb of ram).

[davidfateh]

I am also experiencing a bit of lag when using more than 5 columns. It seems to be Chrome and Firefox are hit the worst and Safari is fairly smooth. This is using the windowscroller and flextable.

[bvaughn]

Haven't been able to profile it much yet. Unfortunately I didn't get back from work / work-function last night until 11pm and I was too tired to look at it at that point. I'll try to make some time to dig into it this evening. Would welcome any help if you get there before me. ;)

[bvaughn]

I did a bit of profiling last night after all. From what I can see, there don't appear to be any big wins. The slowness seems to be coming from all of the features / functionality FlexTable provides. As I strip out features, it gets faster. (For example, stripping out the extra <div> wrapper in each column that truncates text and prevents it from overflowing its flexbox parent speeds things up a bit- but that's an important feature of the table.)

Not done looking. I think there are definitely a couple of things I could precompute that may speed things up a tiny bit. I'm not sure there are any obvious big wins though.

[davidfateh]

Thanks for checking it out. I've forked it and I'm going to take a look as well although it's going to take me a bit longer...

Do you think it's necessary for you to worry about display at your level? Could you leave that more to the end user? Maybe a user of this would actually want overflow or no overflow or some kind of custom overflow like ellipsis. I wonder if leaving that up to the user and having them deal with the performance issues of that is reasonable.

[bvaughn]

Do you think it's necessary for you to worry about display at your level? Could you leave that more to the end user?

Well I guess that depends. On the one hand, FlexTable and VirtualScroll exists _solely_ for the convenience. If a user wants (or is willing to) take on the "display" logic he/she could just use Grid and- in some cases, like this- it would likely be faster.

That being said, I'm not afraid to delegate some of the display work if it could speed things up. Just need to weigh the pros (how much does it improve performance) and cons I guess (loss of convenience, major release required, etc)

[davidfateh]

I've played with the Grid and it is definitely faster and visually not as laggy when using multiple columns. But the use of arrays for rows instead of key/value is not ideal for replacing it with the table seeing as it would be possible for the data to be displayed in the wrong order in any given row.

I understand what the Grid is supposed to be used for but I wouldn't agree that using it as a replacement to the table is useful seeing as I would lose the sorting functionality as well as fixed headers.

I'm going to play around with removing the divs like you had done and see how much more work is required on my end to handle overflow cases.

[bvaughn]

I didn't mean it would be a _drop in replacement_ per say. Rather, since the FlexTable decorates a Grid, it would be possible to achieve the same sort of functionality with a custom decorator (eg MyFlexTable) that cut out features/functionality you didn't require in the vanilla FlexTable.

Still, I want to get as much performance as I can out of FlexTable- so my main hope is that we can find some more wins in it and this side discussion is irrelevant. :)

[davidfateh]

Yes I agree. I'd like to get more performance from FlexTable as I really like the component as a whole. I'm going to take a jab at it this weekend and see if I can find anything as well.

Thanks for all the responses, I appreciate your punctuality on this :)

[bvaughn]

No problem. I'm excited to have some help! :)

I've been working this morning on a testing harness that measures scrolling performance in a way that's more reliable than the adhoc testing I've been using. I'll share it with you if you'd like. It's not very pretty at the moment but it gives me a higher confidence that the changes I'm making are actually having a positive or negative impact.

[davidfateh]

Yes definitely, I'd love to see that. I was going to go straight to the React perf addon for traditional benchmarks.

[bvaughn]

I've found React perf to be a bit confusing. Could be me. Admittedly I haven't spent _much_ time with it.

Here's a gist of what I'm doing:
https://gist.github.com/bvaughn/1fc166d5cb3f8c174a1552eeaeeaa0e6

Needs to be cleaned up before I'd be willing to _really_ share it but it gives a reasonable idea of the average framerate for a table being aggressively scrolled.

Can be pasted into the console and run against playground builds like this: https://rawgit.com/bvaughn/react-virtualized/master/playground/table.html

[bvaughn]

Actually I went ahead and posted it to Git as well so you can try it directly:
https://rawgit.com/bvaughn/react-virtualized/master/playground/table.html

Click anywhere on the page to start the perf test. Once it completes it will print stats to the console.

Test runner utility code here:
https://github.com/bvaughn/react-virtualized/blob/master/playground/test-runner.js

The test case is kind of unrealistic (a 40 column table) but it demonstrates the perf bottleneck nicely.

[bvaughn]

FYI I just realized that my table test harness was pointing at a dev build of React (in CDN but still dev build). Ugh. Updated to use the .min.js (production) build and performance is significantly better. Still room for improvement but much better.

[bvaughn]

I've been able to squeeze an additional 0.5-1 FPS out of FlexTable via PR #282 (from ~41 to ~42, it varies a bit test to test). This table contains 40 columns and is scroll-slammed intentionally to stress the test. Decreasing the column count to 20 increases the FPS to ~54. Decreasing to 10 increases the FPS to 60.

Anyway, stepping away for a bit. Will continue looking.

[bvaughn]

Just a quick update after doing some more experimentation this morning. Tried a couple of different approaches to cell-caching in Grid. All added complexity but made no noticeable improvement in performance.

Did find a pretty big win (increasing FPS by ~1/3) by eliminating the <div className='FlexTable__rowColumn'><div className='FlexTable__truncatedColumnText'>...</div></div> wrapper elements and instead just passing a className and style to cellRenderer and relying on it to set that information on its own rendered content. I don't think that's a very user-friendly change to make to the interface though and it would require, at the least, a major release.

I've seen ~1/4 FPS increase (obviously this is dependent on the type of test harness, the number of columns, etc) by removing just the inner <div className='FlexTable__truncatedColumnText'>...</div> wrapper- but this causes overflow text to be _clipped_ instead of truncated. I have similar reservations about this change in terms of user-friendliness.

[bvaughn]

@davidfateh: Yes definitely, I'd love to see that. I was going to go straight to the React perf addon for traditional benchmarks.

FYI @davidfateh I released the FPS utility I created the other day for measuring this stuff as its own Npm package: https://github.com/bvaughn/fps-measurer

[davidfateh]

Hey admittedly I haven't looked at this yet this weekend but it seems like you've done a lot of work. I thank you for that. Is it possible that instead of using the truncated cell column we can instead use text-overflow: ellipsis;? That may help speed up the process.

Also, another possibility is that you can include a prop that will enable the truncation by default (so you don't need a major release) but add the ability to remove the truncation div and allow for manual user intervention when performance is an issue. Maybe that can be combined with text-overflow: ellipsis;.

[bvaughn]

Is it possible that instead of using the truncated cell column we can instead use text-overflow: ellipsis;

Unfortunately, no. The flex spec doesn't support direct-text content being truncated via ellipsis. (It gets clipped instead.) The only work around to this - at least the only one I'm aware of - is to put a _non_ flex container around the text and set text-overflow: ellipsis; on it. That's why I have 2 <div> in the first place unfortunately. The outer one (flex) handles vertically centering the text and the inner one (block) truncates the text.

[bvaughn]

Good news, not including the above <div> changes, I've still been able to add 3-4 additional FPS in my test harness with the changes made in the branch corresponding to this ticket.

[bvaughn]

Good news! I've been able to roughly double the FlexTable frame rate for my stress test project after several small tweaks and changes in the FlexTable-perf branch. I want to do a bit more testing but I hope to have a backwards-compatible release ready to go out later today.

[bvaughn]

Just realized react-virtualized 7.8.0.

For me, this released dramatically improves frame rate for FlexTables with 20+ columns. I see 10-15 FPS improvement in Chrome and Firefox and near 2x improvement in Safari.

I believe that this, combined with using _production_ React builds for baselines instead of development builds should hopefully help address your concern. As such, I'm going to close this issue for now to keep the issues list tidy. That being said- I'm always looking for ways to squeeze more performance out of the framework- so let's talk more if you have more ideas. :)

[bvaughn]

PS. If you'd like to compare the differences in performance for FlexTable yourself:

• Before: http://rawgit.com/bvaughn/react-virtualized/master/playground/table.html?source=7.7.1
• After: http://rawgit.com/bvaughn/react-virtualized/master/playground/table.html?source=7.8.0

Load the page, click anywhere to start the test, and watch it log FPS info to the console. On my laptop using Chrome 51, v7.7.1 logs 36fps and v7.8 logs 51fps.

Similar test for a vanilla Grid:

• Before: http://rawgit.com/bvaughn/react-virtualized/master/playground/grid-test.html?source=7.7.1
• After: http://rawgit.com/bvaughn/react-virtualized/master/playground/grid-test.html?source=7.8.0

For me on Chrome v7.7.1 has 47.5 fps and v7.8 has 50.6fps. Smaller improvement, but still an improvement.

[bvaughn]

Bummer. Had to revert the tabIndex change to Grid and release as 7.8.1 to avoid breaking keyboard accessibility too badly. This drops a couple of FPS because of focus causing repaints. But.... it's still a marked improvement for FlexTable over all.

[davidfateh]

I definitely see the performance boost. Thanks for taking the time to look into this! I'm going to continue to play with it on my end as I actually do some implementation for the project I'm working on.

[bvaughn]

Huzzah! Glad to hear. :)

I have some more ideas that came to me over night that I'll play with some more. They'll probably be more incremental but every little bit helps.

[davidfateh]

Are you still considering removing the truncation completely and letting the user decide that? Or maybe just have a flag that disabled the truncation and thus removes the need to render the extra div?

[bvaughn]

I was actually able to remove 1 of the 2 wrapper <div>s already. The one that remains is used to set flex grow/shrink/basis properties- which is pretty important.

I _could_ potentially remove that one as well- if people provided a custom renderer. But by default the FlexTable cell renderer just returns a string that's wrapped with this <div> so it's necessary in that case.

[alolis]

@bvaughn , I am playing around with latest react-virtualized (v8.4.0), React (v15.1.0) and Table component, with a similar setup as the OP with 11 columns and 1000 items pre-loaded, in combination with WindowScroller and AutoSizer and I am noticing very poor performance I am afraid.

On the example here, its way smoother (although with very simple cell data) than what I notice on my side:
https://rawgit.com/bvaughn/react-virtualized/master/playground/table.html

Is the code for the above sample available so I can have a look and maybe notice something different from my setup?

I do have a few custom cell renders but they are very simple (they basically convert bytes to human formats, they don't even wrap the value with an HTML element).

I also have an onMouseOver callback and rowClassName in order to change the css style of the currently hovered row and I notice huge lags between current position and the row being painted with my on hover css class color. For example while I hover over the list, i notice that the css class changes for 3-4 rows up instead of the one currently under the pointer.

overscanRowCount={20} but It didn't help much either.

I am noticing this problem under latest Chrome and things are worst under Firefox.
Is there some more info/tests I can provide to help you figure out what's going on because it's kinda unusable at the moment?

UPDATE: Switched to Grid to test it and its very smooth. I need to use Table though, fits better for my case.

[bvaughn]

@alolis Anecdotally, I've noticed when people report performance issues- sometimes the issue is on the app side (eg rendering too frequently b'c you don't have a proper shouldComponentUpdate method, stuff like that). I would guess that something like that is going on on your side- but without seeing your code it's hard to know for sure. Have you tried adding logging in your renderers to make sure they aren't being called more often than you would expect?

On the example here, its way smoother (although with very simple cell data) than what I notice on my side:
https://rawgit.com/bvaughn/react-virtualized/master/playground/table.html
Is the code for the above sample available so I can have a look and maybe notice something different from my setup?

Yup! All of the playground code is here. The source for the specific example you linked to is here.

[alolis]

@bvaughn , I am making a dummy component with the code from the playground. I will not load any other components, just the one from the playground and I will report back.

I already did try to load your sample code from here and I noticed performance issues as well, but just to be sure that it's not something else within my component, i will just make a fresh one with only the minimum and paste the complete code here with my observations. You might be right of course, and simply something silly on my side is causing this, I will have more info very very soon.

Have you tried adding logging in your renderers to make sure they aren't being called more often than you would expect?

I already tried to remove my cell renderers and the performance was the same I am afraid.

[alolis]

@bvaughn , ok, so, I created a fresh route in my app that loads _only_ the component from the playground. I did change it just a little in order to show the structure of the Table rendering I am using in my actual component:

'use strict';

import React, {PropTypes} from 'react';
import {
  Table,
  Column,
  AutoSizer,
  WindowScroller
} from 'react-virtualized';

const NUM_COLUMNS = 40

class TableTest extends React.Component {
  rowGetter(params) {
    return new Array(NUM_COLUMNS).fill('').map(
      (_, index) => {
        return index
      }
    )
  }

  generateColumns() {
    const flexColumns = []

    for (var i = 0; i < NUM_COLUMNS; i++) {
      flexColumns.push(
        React.createElement(
          Column,
          {
            dataKey: i,
            flexGrow: 1,
            key: i,
            width: 50
          }
        )
      )
    }

    return flexColumns;
  }

  render() {
    const flexColumns = this.generateColumns();

    return (
      <div>
        <WindowScroller>
          {({height, scrollTop}) => (
            <AutoSizer disableHeight>
              {({width}) => (
                React.createElement(
                  Table,
                  {     
                    autoHeight: true,             
                    overscanRowCount: 0,
                    rowHeight: 30,
                    rowCount: 1000,
                    rowGetter: this.rowGetter,
                    height,
                    width,
                    scrollTop
                  },
                  null,
                  flexColumns
                )
              )}
            </AutoSizer>
          )}
        </WindowScroller>
      </div>
    )
  }
}

export default TableTest;

Unfortunately I am still not getting the same performance as the sample in the playground but I do notice a few things.

If i get rid of the WindowScroller and change my render to the following, I do get better performance, although still not the same as your sample:

render() {
    const flexColumns = this.generateColumns();

    return (
      <div>
        <AutoSizer>
          {({height, width}) => (
            React.createElement(
              Table,
              {                 
                overscanRowCount: 0,
                rowHeight: 30,
                rowCount: 1000,
                rowGetter: this.rowGetter,
                height: 1000
                width,
              },
              null,
              flexColumns
            )
          )}
        </AutoSizer>
      </div>
    )
  }

However, I do notice two small problems:

1. The height key (params.height in your example) is always 0 so I used a fixed value of 1000 to run the test. Do you see something wrong in the sample above that might be causing the height to be always 0? In addition, if i remove the wrapper div, width is 0 as well. Using React.createElement for AutoSizer as well didn't make any difference.
2. In your example I only see the browser window scrollbar, in my sample I see the container scrollbar as well. How do you achieve this without the WindowScroller?

Note: I updated to 8.4.1 before doing the above tests and also switched to React 1.5.2, which is the same version you use in your samples.

[bvaughn]

1. The height key (params.height in your example) is always 0 so I used a fixed value of 1000 to run the test. Do you see something wrong in the sample above that might be causing the height to be always 0? In addition, if i remove the wrapper div, width is 0 as well. Using React.createElement for AutoSizer as well didn't make any difference.

In general, when this happens, it means that one or more ancestor/parent DOM elements (eg a div or the body tag) don't have height: 100% and so AutoSizer isn't able to grow. To test this, try adding background-color: red to each of the ancestors individually. The first time you don't see a red background- you've found the one (or one of the ones) that needs it height to be set via CSS.

Looking at the code you pasted above, the outer <div> element would need to have its height set for example. Divs aren't 100% tall by default, only wide.

That being said, the wrapper <div> should not be necessary at all and may be causing both of your problems. Try removing it?

[alolis]

@bvaughn , thanks for the recommendation. Indeed it's probably something related, I will figured out.

Unfortunately the performance though is an issue even I stripped down everything and used the code I showed you above, which is taken from your examples. Regarding the WindowScroller, it seems that there is an older issue about it as well #281

Is there something more I can do on my side to provide information that might help you to figure out if it's a problem or not?

[bvaughn]

The WindowScroller issue you linked to has already been fixed, so it's
unlikely to be related.

On Nov 7, 2016 6:03 AM, "Alexander Lolis" [email protected] wrote:

@bvaughn https://github.com/bvaughn , thanks for the recommendation.
Indeed it's probably something related, I will figured out.

Unfortunately the performance though is an issue even I stripped down
everything and used the code I showed you above, which is taken from your
examples. Regarding the WindowScroller, it seems that there is an older
issue about it as well #281
https://github.com/bvaughn/react-virtualized/issues/281

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/bvaughn/react-virtualized/issues/278#issuecomment-258843101,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABznRT04-QeqM5L253pyyMbl-O7_Hxyks5q7y_BgaJpZM4I2b18
.

[alolis]

Yes i am aware, but since there was already a problem and I still notice it some bug might still be around; they do have the tendency to cluster around the same code block! :)

I will keep playing around and provide more feedback.