Workiva / Workiva/over_react

connect creates new Props map object on every Action dispatch, which must be deeply compared

Open
#434 22 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Dart
Stars
432
Forks
57
PR merge metrics
No merged PRs in 30d

Description

  • Issue Type: FEATURE REQUEST
  • over_react Version(s): 3.1.5

I think that the performance of the connect function in OverReact redux, specifically the performance of the functions it sets up such as handleAreOwnPropsEqual, handleAreStatePropsEqual, etc. is adversely affected by potentially unnecessarily equality testing.

I can't set up a simple example to show the effect. It's just something I notice in my complex app with fairly large State. I'm trying to do very frequent updates (drawing a box created by the user click-dragging), 60 times per second. It can barely render more than once or twice a second when the State is large, even though it is only updating one tiny little object in the State tree representing the dimensions of the box.

I've done the recommended React/Redux optimizations such as setting up connect on components that would take a while to render, and writing shouldComponentUpdate on unconnected components. I've profiled with React DevTools and I know that no components are re-rendering that don't need to, so I know it is not unnecessary re-renders making it slow. According to React DevTools, the total render time for everything is very fast (about 1 ms in each round up updating).

The Chrome performance profiler is tough to pick apart, because it points to execution in dart_sdk.js, and who knows which Dart code caused that call.

But the profiler tells me that each round of updates takes at least 100 ms (preventing 60 Hz rendering), and 70% of this time is being spent in _shallowMapEquality in src\over_react_redux\over_react_redux.dart. I stepped through and found that (a representation of) my entire State tree is being deeply compared for equality using this function every time any Action is dispatched.

I was very careful to set up an immutable State using built_value and built_collection, only generating a new State (or new part of a subtree of the State) if something changes. I even re-wrote parts of built_value to cache hashCode so that it is not re-computed unnecessarily. I am also sure that my reducers, when they don't change any data, return the same object, so for such objects an equality comparison should be a one-line call to identical(this, other).

But somewhere in the depths of OverReact Redux, I think these carefully constructed objects are being translated into Js-backed Maps, where two different map objects exist that represent the same data, and they need to be deeply compared all the way down to the leaves, just to check that they are equal.

I could be wrong about this, because I don't really understand the architecture of OverReact Redux. But perhaps there is a way to do some optimizations within OverReact Redux so that when data in most of the State tree does not change, the same objects are kept in place so equality testing is a fast call to identical and does not need to recurse deeply.

Alternately, do you have a suggestion for how to structure my code to prevent such unnecessary equality checking?


FYI: @greglittlefield-wf @aaronlademann-wf @kealjones-wk @evanweible-wf @maxwellpeterson-wf

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with src\over_react_redux\over_react_redux.dart and inspect _shallowMapEquality and the connect handlers mentioned in the report. Profile the reported large-State, frequent-dispatch case to confirm where new map objects and deep comparisons arise; done should mean unchanged state can be recognized without recursively comparing the full tree.

Written by the indexing model from the issue text.

Assessment

Tech stack
dart, react
Domain
frontend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.