Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slow Performance Observed When Updating State On Keyup #691

Closed
cdaringe opened this issue Sep 5, 2015 · 18 comments
Closed

Slow Performance Observed When Updating State On Keyup #691

cdaringe opened this issue Sep 5, 2015 · 18 comments
Labels

Comments

@cdaringe
Copy link

cdaringe commented Sep 5, 2015

Hi all:

problem statement

On each keystroke trigger an action, reduce the state to compensate for the new character, but observe that re-rendering becomes laggy. You can watch the lag here.

background

  • I use react-router, and have a simple component structure (I'd snap a shot of the component DOM, but electron react-dev tools is currently out-of-operation). It's a few layers deep:
Provider >
App >
Dashboard  >
Home {connected, but passes no state/props through children. why? to dispatch an auth middleware event on load} >
ProjectDashboard >
ProjectController {connected, passes a single object of state, ~small} >
Project {dumb}

a third component is connected, but is not rendered. its a layout component for login, and I don't believe to be associated with the issue at hand.

I've profiled while punching keys into the field. It's the name field shown. It looks like a ton of time is being spent in validate and doing deepEquals in shouldComponentUpdate.
screen shot 2015-09-04 at 6 25 01 pm
screen shot 2015-09-04 at 8 34 33 pm

discussion

  • it's been noted to move components down lower in the structure if perf is an issue, so I've done that. i only have a few components, and they pass the minimal amount of state via connect
  • my reducers and state updating is waaay down the list of processing % time. it seems to correspond to actual react re-rendering time in some components, perhaps performing frivolous checks?

Would love some tips! Thanks!

@cdaringe cdaringe changed the title Slow Performance Observed When Updating State On Keystroke With Single Connected Component Slow Performance Observed When Updating State On Keystroke Sep 5, 2015
@cdaringe cdaringe changed the title Slow Performance Observed When Updating State On Keystroke Slow Performance Observed When Updating State On Keyup Sep 5, 2015
@timdorr
Copy link
Member

timdorr commented Sep 5, 2015

What is contained in props on your Project component? It looks like you've got something big in your props that's being validated.

I'd recommend grabbing the react devtools, as it makes viewing this stuff much easier.

Could you post your Project component's code?

@cesarandreu
Copy link
Contributor

I get similar performance issues with forms when I'm using redux-devtools.

Have you tried out reselect? It'll memoize selectors, which should help you avoid re-calculating stuff when data changes.

Aside from that, pulling in react-pure-render might help as well.

@cdaringe
Copy link
Author

cdaringe commented Sep 6, 2015

hi @timdorr @cesarandreu. thanks for your willingness to add ideas!

  1. my selected-mapped state isn't tiny, but it's not huge either. see here. I could perhaps reduce some of that data further. do you know much about the what and why of the validation step?
  2. i have tried to get react devtools up, but it's currently out-of-order in electron, which is my target client.
  3. in regards to reselect, i'm not modifying as I select my state-bits into the component's stateToProps map. but perhaps therein is part of the issue? i'm still reasoning over why you'd cache in this part of the processes vs. before/conditionally dispatching an action.
  4. i will be trying pure render for sure
    thanks guys! would definitely care for some more tips if you got them!

@gaearon
Copy link
Contributor

gaearon commented Sep 6, 2015

It's a known issue with Redux DevTools and if you'd like to see it fixed, a good first step is to contribute tests. You won't have this issue in production.

@gaearon gaearon closed this as completed Sep 6, 2015
@gaearon
Copy link
Contributor

gaearon commented Sep 27, 2015

There's a PR attempted to fix it: reduxjs/redux-devtools#133
Can you please test whether it improves the situation?

@gaearon
Copy link
Contributor

gaearon commented Sep 27, 2015

This is fixed in redux-devtools@2.1.5.
Performing an action with DevTools open was O(n), now it's O(1).

@cdaringe
Copy link
Author

thanks @gaearon! will try at my next opportunity

@andy-polhill
Copy link

I'm still seeing a bit of a lag when carrying out rapid state changes on the latest version (in this scenario it's a keydown event for an autocomplete). My suspicion is that it may be due to the number of LogMonitor nodes it has to update in quick succession. I created a very simple custom monitor which just logged the action and the problem seemed to go away. If anyone else is experiencing this I think the resolution may be one of the following..

  • Create a simple custom monitor with an emphasis on speed
  • In the default LogMonitor raise a PR to only retrieve action details on click

(I can hopefully help out with a resolution, I wonder if this issue could be transferred to redux-devtools)

@gaearon
Copy link
Contributor

gaearon commented Oct 1, 2015

Oh, I see now. There hasn't been much effort to optimize LogMonitor so I wouldn't be surprised if there are many low hanging fruit there.

@davecoates
Copy link

Adding a shouldComponentUpdate that does a shallowEqual for LogMonitorEntry seems to greatly improve responsiveness for me when using redux-forms (goes from unusable to usable). I'm not entirely sure if it's safe to just do a shallowEqual though... so far seems ok.

@gaearon
Copy link
Contributor

gaearon commented Oct 9, 2015

@davecoates Then please file an issue (or, better, a PR) to add it!

@vincentfretin
Copy link
Contributor

@davecoates The issue I found with redux-form is that onChange', 'onBlur', 'onFocus', 'onUpdate',
'handleChange', 'handleBlur', 'handleFocus' on field prop change on every render. I put a shouldComponentUpdate on my Widget component (that render a input, select or whatever) to ignore changes of those functions. This is what I use:

const IGNORE_FUNCTIONS = [
  'onChange', 'onBlur', 'onFocus', 'onUpdate',
  'handleChange', 'handleBlur', 'handleFocus']


export function shallowEqual(objA, objB) {
  if (objA === objB) {
    return true
  }

  if (typeof objA !== 'object' || objA === null ||
      typeof objB !== 'object' || objB === null) {
    return false
  }

  const keysA = Object.keys(objA)
  const keysB = Object.keys(objB)

  if (keysA.length !== keysB.length) {
    return false
  }

  // Test for A's keys different from B.
  const bHasOwnProperty = Object.prototype.hasOwnProperty.bind(objB)
  for (let i = 0; i < keysA.length; i++) {
    if (!bHasOwnProperty(keysA[i])) {
      return false
    }
    if (keysA[i] === 'field') {
      if (!shallowEqual(objA[keysA[i]], objB[keysA[i]])) {
        return false
      }
    } else if (typeof objA[keysA[i]] === 'function' &&
      IGNORE_FUNCTIONS.indexOf(keysA[i]) > -1 ) {
      // don't compare function is the IGNORE_FUNCTIONS array
      continue
    } else if (objA[keysA[i]] !== objB[keysA[i]]) {
      // console.log('rerender because of prop named:')
      // console.log(keysA[i])
      // console.log('before:')
      // console.log(objA[keysA[i]])
      // console.log('after:')
      // console.log(objB[keysA[i]])
      return false
    }
  }

  return true
}


export default function shouldPureComponentUpdate(nextProps, nextState) {
  return !shallowEqual(this.props, nextProps) ||
         !shallowEqual(this.state, nextState)
}

class Widget extends Component {
  static propTypes = {
    field: PropTypes.object.isRequired,
  }
  shouldComponentUpdate = shouldPureComponentUpdate
}

@gaearon
Copy link
Contributor

gaearon commented Oct 10, 2015

@vincentfretin Please file an issue in Redux Form. You shouldn't have to do these optimizations by hand.

@gaearon
Copy link
Contributor

gaearon commented Oct 17, 2015

Redux DevTools 3.0 Beta 2 should vastly improve performance.
Please help test it: https://github.com/gaearon/redux-devtools/releases/tag/v3.0.0-beta-2

@stanleycyang
Copy link

I'm still seeing the same issue in 3.0.5 for onChange.

@gaearon
Copy link
Contributor

gaearon commented Dec 23, 2015

@stanleycyang Please share a project reproducing the problem.

@fingermark
Copy link

Edit: this is for DevTools. Just noticed this is in the redux ticket, as it was the first thing that popped up when I googled the two terms. I can move it if you'd prefer.

git clone https://github.com/erikras/redux-form
git checkout gh-pages
npm install
npm run dev

Navigate to http://localhost:3030/#/examples/synchronous-validation

Hold down the spacebar in the username field

Remove the DevTools from the project: src/index.js by deleting the following line

{devToolsEnabled && !window.devToolsExtension && <DevTools/>}

Refresh

Notice speedup.

You can also get a preview here: http://erikras.github.io/redux-form/#/examples/synchronous-validation?_k=hl6zzk

This is not just keyup for me. DevTools is slow for everything.

@crobinson42
Copy link

90% of folks that experience this issue aren't building their react bundle/compile with the production flag on. And another interesting fact of the day, 99% of statistics like my statement are made up! Haha

Best resolution if your building your React app code with grunt or something:

export NODE_ENV=production && grunt

That usually does the trick, IME

@reduxjs reduxjs deleted a comment from m0o0scar Jun 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

10 participants