search

mobxjs / mobx-react

React bindings for MobX
https://mobx.js.org/react-integration.html
MIT License
4.85k stars 349 forks source link

js framework benchmark - Huge performance drop after upgrading mobx and mobx-react to latest version #877

Closed darkowic closed 4 years ago

darkowic commented 4 years ago

After upgrading react, mobx and mobx-react to latest version in https://github.com/krausest/js-framework-benchmark/tree/master/frameworks/keyed/react-mobX I found huge performance drop in the benchmark. The example is really simple but I don't know what is the problem.

image

How to reproduce the issue

Upgrade react, mobx and mobx-react to the latest version and run the benchmark.

Versions

  "dependencies": {
    "mobx": "5.15.4",
    "mobx-react": "6.2.2",
    "react": "16.13.1",
    "react-dom": "16.13.1"
  }
mweststrate commented 4 years ago

Shit, yes forget about this.

@FredyC I think we still need to revert the batching-opt-in change? We really can't do that in a minor update, it hurts serious applications really bad and probably no test will detect it.

danielkcz commented 4 years ago

@darkowic So you haven't noticed that "friendly" warning what you need to do?

darkowic commented 4 years ago

Yeah, I didn't notice any warning

danielkcz commented 4 years ago

And can you please confirm that after following these instructions the performance is back on track?

darkowic commented 4 years ago

I just checked it. Adding require('mobx-react/batchingForReactDom'); didn't solve it. [Violation] 'click' handler took 34428ms - creating 10k rows...

You have to take a look at this benchmark I guess.

danielkcz commented 4 years ago

Alright, thank you for the report in that case and we will have a closer look.

There are fairly big jumps between versions for mobx (5.0.3 -> 5.15.4) and mobx-react (5.2.3 -> 6.2.2). It will be an interesting discovery path for sure :) Might be a problem of MobX itself.

mweststrate commented 4 years ago

Just tried to narrow this done, it seems that the react update itself causes the difference. As soon as I upgrade to react 6.13.1, the perf drops from 3 to 40 secs, with the same mobx / mobx-react version. Does this ring a bell to anyone?

Edit: drop happens between react 6.5.2 and 6.6.0

Edit: looks like we need to update some build deps: https://github.com/facebook/react/issues/13987

Edit: using Terser fixes the issue indeed.

darkowic commented 4 years ago

This is mindblowing :exploding_head: I would not expect minifier to do that... Thank you for the investigation!

mweststrate commented 4 years ago

Yeah, it's super sad! Glad someone else found that out before, would never have figured that out myself.