esnet / react-timeseries-charts

Declarative and modular timeseries charting components for React
http://software.es.net/react-timeseries-charts
Other
865 stars 283 forks source link

Performance issues when data remains consistent, but tracker position changes #322

Open AlanFoster opened 6 years ago

AlanFoster commented 6 years ago

Hey, I've noticed the performance for react-timeseries-charts suffers when the tracker position changes - but the underlying dataset is constant.

As a demo - this is the simple timeseries I'm working with, it's a slightly artificial data-set but it shows the main point of being slow to respond when moving the cursor around:

https://codesandbox.io/s/rwo1y6qqjm

It would be great if there's a way to improve the performance under this scenario

AlanFoster commented 6 years ago

Phew; After poking about for a bit in the source code, I've worked out that although the YAxis component is smart and avoids re-rendering when possible. Unfortunately the ScatterChart component isn't as smart, and would need similar logic to improve performance.

AlanFoster commented 6 years ago

Just noticed an additional issue:

Using series.columns() passed in as a prop to LineChart caused re-renders too. Checking out the implementation I noticed that it currently relies on strict equality for its shouldComponentUpdate implementation:

var columnsChanged = this.props.columns !== columns;

Checking out the implementation I can why it's currently forcing rerenders:

columns() {
    const c = {};
    for (const e of this._collection.events()) {
        const d = e.toJSON().data;
        _.each(d, (val, key) => {
            c[key] = true;
        });
    }
    return _.keys(c);
}

https://github.com/esnet/pond/blob/02281b13b4667781af6034522ab1fd5fa5004077/src/pond/lib/timeseries.js#L531-L540

Since pondjs is immutable, maybe series.columns() could always return the same array reference? 🤔

pjm17971 commented 6 years ago

These are great observations. Thanks for digging into this! 👍

hannak016 commented 3 years ago

Hello, I am wondering if this has been solved? Cheers!