React: Bug: v17 DEV mode slowness during effect traversal

[joacub]

React version: 17.0.0.rc2

Steps To Reproduce

1. Install in dev mode
2. all events, click, onBlur all of them are too slowly every time you click in a input or something freeze the browser

And even more if you open the chrome dev tools or others becomes more slow

[bvaughn]

It doesn't look like this bug report has enough info for one of us to reproduce it.

Please provide a CodeSandbox (https://codesandbox.io/s/new), a link to a repository on GitHub, or provide a minimal code example that reproduces the problem. Screenshots or videos can also be helpful if they help provide context on how to repro the bug.

Here are some tips for providing a minimal example: https://stackoverflow.com/help/mcve

[bvaughn]

cc @trueadm

[bvaughn]

@joacub Are there any DEV warnings in the console during this period of slowness?

[joacub]

@joacub Are there any DEV warnings in the console during this period of slowness?

No there is no warnings, i do a js profiler and react call severals times to one same function over and over, i will share this soon

[joacub]

this is related
https://github.com/facebook/react/issues/19958

[joacub]

here is the profiler:

and there is more and more calls to the same function and the other function discreteUpdates too take long:

[bvaughn]

That is a recursive function that React calls once per component with mutation effects (e.g. add <div> to the DOM).

this is related

19958

Possibly, though hard to say with confidence yet.

[joacub]

That is a recursive function that React calls once per component with mutation effects (e.g. add <div> to the DOM).

this is related

19958

Possibly, though hard to say with confidence yet.

like 1000 times ? what do you mean mutation effects? this ---> document.appendChild or what ?

this works perfectly in production and in safari and when the dev tools is close works better but still slowly.

im even more im sure im not using mutation effect like document.appendChild if yo are talking about that.

[gaearon]

@joacub Brian is saying it is absolutely expected to see that function in the stack trace a lot — basically, once per <div> or a component that you're mounting. What would not be expected is seeing it more times than the nesting of your tree. It's hard to say anything from your screenshots — it would really help if you could either isolate the problematic behavior in an example or share your project or deploy it somewhere.

[joacub]

@joacub Brian is saying it is absolutely _expected_ to see that function in the stack trace a lot — basically, once per <div> or a component that you're mounting. What would not be expected is seeing it more times than the nesting of your tree. It's hard to say anything from your screenshots — it would really help if you could either isolate the problematic behavior in an example or share your project or deploy it somewhere.

project is private so I can't share, but I understand what he said, im not remounting all tree is just one button nothing else, that's happens in all click events, all of my components are using React.memo with () => true always so never has to be remounted or called. and more that click is in one separate element which is in outside of my tree child components.

before react 17 this was working pretty good now is unstable 100%, I can't even work

[bvaughn]

A little more context: Even if only one component in your tree has a mutation effect, React will recurse from the root down to that component.

         A
      B     C
   D           E
F     G

Let's say G has a mutation effect, React would still recurse from A -> B -> D -> G to apply the effect. The deeper your tree is, the taller the call stack would be.

before react 17 this was working pretty good now is unstable 100%, I can't even work

I don't know what this means.

It's very difficult for us to guess about what might be causing this issue without a repro. If you can't share your private app with us, can you make a reduced case that shows the problem on Code Sandbox?

[gaearon]

project is private so I can't share

Is it deployed somewhere in production that we could access? E.g. maybe it's a live website.

[joacub]

project is private so I can't share

Is it deployed somewhere in production that we could access? E.g. maybe it's a live website.

yes but production works perfect, just in dev mode is super slow.

you can access here bringerparcel.dev

[joacub]

A little more context: Even if only one component in your tree has a mutation effect, React will recurse from the root down to that component.

         A
      B     C
   D           E
F     G

Let's say G has a mutation effect, React would still recurse from A -> B -> D -> G to apply the effect. The deeper your tree is, the taller the call stack would be.

before react 17 this was working pretty good now is unstable 100%, I can't even work

I don't know what this means.

It's very difficult for us to guess about what might be causing this issue without a repro. If you can't share your private app with us, can you make a reduced case that shows the problem on Code Sandbox?

what I mean with this

before react 17 this was working pretty good now is unstable 100%, I can't even work

is... in react 16.3.1 this issue does not exist

[bvaughn]

yes but production works perfect, just in dev mode is super slow.

you can access here bringerparcel.dev

That is a production build, so it doesn't really help us reproduce the problem.

[joacub]

A little more context: Even if only one component in your tree has a mutation effect, React will recurse from the root down to that component.

         A
      B     C
   D           E
F     G

Let's say G has a mutation effect, React would still recurse from A -> B -> D -> G to apply the effect. The deeper your tree is, the taller the call stack would be.

before react 17 this was working pretty good now is unstable 100%, I can't even work

I don't know what this means.

It's very difficult for us to guess about what might be causing this issue without a repro. If you can't share your private app with us, can you make a reduced case that shows the problem on Code Sandbox?

i understand what react is doing but this is normal to take like almost 3 seconds per click ? that's not normal.

[joacub]

i have no enough time for reproduce this in code sandbox, but how I said, the error is very simple, every click event is taking forever

[joacub]

@joacub Brian is saying it is absolutely _expected_ to see that function in the stack trace a lot — basically, once per <div> or a component that you're mounting. What would not be expected is seeing it more times than the nesting of your tree. It's hard to say anything from your screenshots — it would really help if you could either isolate the problematic behavior in an example or share your project or deploy it somewhere.

I understand, but even If I using React.memo in my component ? I guess yes, but im sure I don't have that nest tree at all, that's insane, I can't even reach the last call function in the tree, is endless.

[bvaughn]

i understand what react is doing but this is normal to take like almost 3 seconds per click ? that's not normal.

This discussion isn't really productive without any way for us to repro the problem. We're kind of just going around in circles.

We have been using what will be version 17 of React at Facebook for several weeks (it changes a little each week) and we have not seen the behavior you're describing. So I would agree that it's not normal.

[joacub]

i understand what react is doing but this is normal to take like almost 3 seconds per click ? that's not normal.

This discussion isn't really productive without any way for us to repro the problem. We're kind of just going around in circles.

We have been using what will be version 17 of React at Facebook for several weeks (it changes a little each week) and we have not seen the behavior you're describing. So I would agree that it's not normal.

I will try get some time for reproduce this in codesandbox

[joacub]

i have n time for share in code sandbox but I test in my environment and I removed all the components in the tree and just leave just one, the behavior is the same super slow every onClick event, took the same time always, this is not related to the mutation effect in the tree, this is related to the events handler or something, how I said and for be clear, I leave just some components and the behavior is the same endless recursively calls.

[joacub]

i did more tests in my project removing more and more dev tools, is just react who has the issue, there is something related to state changes or events handler, sorry if I not been much useful but I have no much time for it. I Appreciate your concern and work.

[gaearon]

Since you’re saying this is DEV-only, this information so far is not helpful without a project we can run locally.

I understand you can’t extract it to a sandbox, but maybe you could put a reduced version on GH?

[Plitzi]

@gaearon i've sent you an email to schedule a moment to test it with ngrok to give you access to localhost

[joacub]

@gaearon if you are able to do the test via ngrok we can do now.

[gaearon]

Awesome, thanks. I'm not available now but I'll respond later.

---

I think I actually have a guess about what's going on. I think we're calling invokeGuardedCallback a lot more times than before.

Looking at RC1, we had code like this:

    do {
      {
        invokeGuardedCallback(null, commitLayoutEffects, null, root, lanes);

        if (hasCaughtError()) {
          if (!(nextEffect !== null)) {
            {
              throw Error( "Should be working on an effect." );
            }
          }

          var _error2 = clearCaughtError();

          captureCommitPhaseError(nextEffect, nextEffect.return, _error2);
          nextEffect = nextEffect.nextEffect;
        }
      }
    } while (nextEffect !== null);

but the actual commitLayoutEffects implementation had its own loop, avoiding repeated invokeGuardedCallback calls:

function commitLayoutEffects(root, committedLanes) {


  while (nextEffect !== null) {
    setCurrentFiber(nextEffect);
    var effectTag = nextEffect.effectTag;

    if (effectTag & (Update | Callback)) {
      var current = nextEffect.alternate;
      commitLifeCycles(root, current, nextEffect);
    }

    {
      if (effectTag & Ref) {
        commitAttachRef(nextEffect);
      }
    }

    resetCurrentFiber();
    nextEffect = nextEffect.nextEffect;
  }
}

Whereas in RC2, we recursively call commitLayoutEffects which wraps every effect into its own invokeGuardedCallback:

function commitLayoutEffects(firstChild, root, committedLanes) {
  var fiber = firstChild;

  while (fiber !== null) {
    if (fiber.child !== null) {
      var primarySubtreeFlags = fiber.subtreeFlags & LayoutMask;

      if (primarySubtreeFlags !== NoFlags) {
        commitLayoutEffects(fiber.child, root, committedLanes);
      }
    }

    {
      setCurrentFiber(fiber);
      invokeGuardedCallback(null, commitLayoutEffectsImpl, null, fiber, root, committedLanes);

      if (hasCaughtError()) {
        var error = clearCaughtError();
        captureCommitPhaseError(fiber, fiber.return, error);
      }

      resetCurrentFiber();
    }

    fiber = fiber.sibling;
  }
}

I can easily imagine this adding a lot of DEV overhead since it happens for every Fiber including host ones.

On WWW, invokeGuardedCallback is forked to go through a fancy try-catch, which is why we did not see it.

Seems like an easy fix. I'll look tomorrow.

[Plitzi]

@gaearon legend! is that

[joacub]

@gaearon 🥳🥳

[joacub]

@gaearon legend! is that

can you please get in touch with me ? contact me at [email protected]

[bvaughn]

Seems like we (React team) need to verify if the guarded callback is the source of slowness. We should be able to do that by disabling our internal "fancy try-catch" :smile: We'll keep this thread posted.

[joacub]

Seems like we (React team) need to verify if the guarded callback is the source of slowness. We should be able to do that by disabling our internal "fancy try-catch" 😄 We'll keep this thread posted.

do you have any ideas or we can change something really quick for at least mitigate for now this ?, _I did try somethings but no successfully._

[bvaughn]

To "mitigate" it? I believe this is only happening in DEV mode and only if you're using an RC, so if it's blocking your local development, don't use the RC until we've released another one.

[Plitzi]

@bvaughn well i have the same issue, i had to back to 16.13.1 and works perfectly is only since 17

[joacub]

@bvaughn in my case I can not go back, there is too many change in my app in more than 400 files, which is will cause me delays so, if you can suggest something for hot FIX this in the meantime I wait for the next RC I will appreciate.

Thanks

[Plitzi]

@joacub try to change to 16.13.1 in your package.json, RC versions supose to not be used in production, that one works perfectly i tested it

[joacub]

@joacub try to change to 16.13.1 in your package.json, RC versions supose to not be used in production, that one works perfectly i tested it

I know, there is no option for me to go back now, that will be a waste or time for me. 😢

[bvaughn]

I don't understand how you have 400 files changed in a way that wouldn't work with v16. v17 doesn't add new APIs or behaviors. Why can't you just pin to a different version of React?

[joacub]

I don't understand how you have 400 files changed in a way that wouldn't work with v16. v17 doesn't add new APIs or behaviors. Why can't you just pin to a different version of React?

The new jsx runtime, i have no anymore in my scope react on these file so i have to do a massive change, i can do with the IDE but i don’t think is going to save me time now, how i said that will be a waste of time

[bvaughn]

Just step back down to RC1 then. We'll release a new RC soon and you can try it again.

[joacub]

Just step back down to RC1 then. We'll release a new RC soon and you can try it again.

I did, is slowly too 😭

[joacub]

Just step back down to RC1 then. We'll release a new RC soon and you can try it again.

just fyi, rc.1 is just a littler better, not much, still been slowly but not than rc.2

[gaearon]

If you have no way to revert, you should not be upgrading to unstable versions. You should use source control and revert undesirable changes, or better yet, not merge them into master in the first place. I'm going to temporarily lock this thread because it's getting very noisy and we're not getting valuable information from it. We'll comment when we figure out the issue.

[gaearon]

We've been able to reproduce the problem, so thanks for the report.

For now we've cut RC3 which doesn't have this issue, so please upgrade to get the fix.

In general though you should never place yourself in a situation where you "need" a bugfix for an unstable release. If you were not prepared to roll back to 16, you should not have run the codemod or upgraded to 17 RC. The whole point of RCs is that they may contain bugs, so they're not suitable for production deployments. And if you decide to ship it to users despite the risk, as soon as there's an issue you need to be prepared to roll back using version control.

[joacub]

We've been able to reproduce the problem, so thanks for the report.

For now we've cut RC3 which doesn't have this issue, so please upgrade to get the fix.

In general though you should never place yourself in a situation where you "need" a bugfix for an unstable release. If you were not prepared to roll back to 16, you should not have run the codemod or upgraded to 17 RC. The whole point of RCs is that they may contain bugs, so they're not suitable for production deployments. And if you decide to ship it to users despite the risk, as soon as there's an issue you need to be prepared to roll back using version control.

Thanks, yes there is no problem, i go back to 17 rc 1, we are working with the last version because we want to release with the last version, thats does not block us at all, yes we run codemon and thats was the only mistake, but even with that we still wanting this, we are in dev stage so we can do and is what we want. Thanks for your work , I appreciate

[gaearon]

Can you verify whether RC3 also resolves the issue for you?

[joacub]

Can you verify whether RC3 also resolves the issue for you?

Yes the issue was resolved, is working great now, thank you so much

[Plitzi]

@gaearon thank you i will take a look too