Possible performance regression in v5

[necolas]

Hi, this is just a heads up. We tried upgrading Twitter Lite to use v5 but noticed that Tweet update timings increased significantly after we deployed to production. Reverting to v4 caused those timings to fall back to the baseline level. We haven't looked into the cause yet.

/cc @paularmstrong

[markerikson]

Really. That's... disturbing.

The benchmarks in #407 and #416 , and other testing, indicate that v5 should be faster for almost all use cases. I think there were one or two exceptions where v4 was faster - think the discussion/examples were in https://www.reddit.com/r/reactjs/comments/5hf4d4/an_artificial_example_where_mobx_really_shines/ and dtinth/pixelpaint#1 .

Any info you can provide would be greatly appreciated.

[jimbolla]

I'd like to help with this if you can provide more info.

[paularmstrong]

I'm looking at narrowing down the cause, but it may take a long time and it's not really a priority at the moment. We only have some perf timings around Timelines and Tweets for "time from construct to mount" and "time from willUpdate to didUpdate". This is the graph of p95 and p50 for update timings:

(Ignore the noise toward the right on p95, that happens periodically) Please keep in mind that this could be something specific with our setup as well, though we're not doing anything special other than using connect with reselect selectors across many components.

[markerikson]

Any further info you can offer on these performance regressions?

[timdorr]

If @necolas or @paularmstrong could offer a flame graph of the app under 5.0, that would be helpful.

[gaearon]

@necolas Have you had a chance to look at this again yet?

[necolas]

No, not yet

[paularmstrong]

Okay, I've finally gotten some things together that are able to show this.

I've created a component that will run 50 updates to a tree and estimate the amount of time each update takes. I did this because I want to be able to do this in a production build of React and not need to rely on react-addons for performance measurements. The implementation of this shouldn't matter at the moment, but I hope to decouple it a bit and open source it in the next week or so.

Specs

• System: Mac OS X
• Chrome: 60.0.3112.113
• CPU: 3.1GHz Intel Core i7 (simulated 10x slowdown in Chrome)
• Packages:
  • reselect@2.5.3
  • react@15.5.4
  • react-dom@15.5.4

Note: These timings are much more apparent on mobile browsers, but they still show up on desktop Chrome enough that these recordings should be good enough

React Redux v4.4.8

Chrome DevTools Timeline

---

React Redux v5.0.6

Chrome DevTools Timeline

---

Component Tree

The following is the connected component tree that I'm rendering in the above recordings.

import { connect } from 'react-redux';
import { createSelector } from 'reselect';
import React, { Component } from 'react';

const exampleMapStateToProps = createSelector(
  (state, props) => 'foobar',
  (foo) => ({ foo })
);

const foobar = () => {};
const exampleMapDispatchToProps = { foobar };

class Internal extends Component {
  render() {
    return <div>barfoo</div>;
  }
}

class InternalContainer extends Component {
  render() {
    return <Internal />;
  }
}

const InternalContainerConnected = connect(exampleMapStateToProps, exampleMapDispatchToProps)(
  InternalContainer
);

class Example extends Component {
  render() {
    return <InternalContainerConnected />;
  }
}

class ExampleContainer extends Component {
  render() {
    return <Example />;
  }
}

export default connect(exampleMapStateToProps, exampleMapDispatchToProps)(ExampleContainer);

[markerikson]

Unfortunately haven't had time to swing back and look at this myself. @jimbolla , you had a whole bunch of benchmarks when we did the v5 rewrite. Any ideas on how this scenario compares so differently to those?

[paularmstrong]

@jimbolla @markerikson any update on this? I can help narrow down traces if needed, but a bit too busy with other things to dive much deeper into react-redux itself.

[jimbolla]

I haven't done anything concrete. I'm not sure how to make it any faster for React 15. Looking at the changes for React 16, it's might be possible to make it faster for that, but slowing down 15. I haven't tested this theory at all though.

[paularmstrong]

@jimbolla we found the same results after upgrading to react 16

[jimbolla]

Yeah. I'd expect that. What I meant was that if we make a version of React Redux that targets React 16's new capabilities, we ought to be able to improve perf. I made an untested proof of concept here: #856. I haven't had a chance yet to put it through its paces, but if this POC moves forward, would you be willing to test it in your environment?

[paularmstrong]

@jimbolla absolutely. Just let me know!

[cellog]

@paularmstrong can you post the part of the benchmark app that does updating? I'm trying to port your example connected component into the performance test suite @markerikson wrote. I just want to be sure I'm actually catching the behavior you found, so we can first verify that 5 is slower.

Did you use this repo?

https://github.com/necolas/react-component-benchmark

[paularmstrong]

@cellog I used/wrote the source that's forked from: https://github.com/paularmstrong/react-component-benchmark

[cellog]

@paularmstrong I need more info. What is the createStore and so on look like? How are you setting up the default run of the benchmark? What props are passed to the sample component? What exactly are you trying to measure? I can't reproduce a benchmark that shows any difference in versions

[paularmstrong]

@cellog I'm trying to create some reduced test cases in codesandbox, but suddenly not able to see the issue anymore. I'll need a few days to test this out at Twitter as well to see if perhaps the issue is mysteriously gone.

Work-in-progress, slowly adding more things from our setup:

• react-redux@5: https://codesandbox.io/s/rm962yrn8p
• react-redux@4: https://codesandbox.io/s/0yq426pq0p

[cellog]

really appreciate the work, thank you

[markerikson]

I know this issue is closed, but I see that Twitter is still using v4.

I'd love to know if v7 is an improvement!