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

Chrome: "Not optimized: optimized too many times" for _updateDOMProperties and unmountChildren #9208

Closed
lencioni opened this issue Mar 17, 2017 · 4 comments

Comments

@lencioni
Copy link
Contributor

lencioni commented Mar 17, 2017

React 15.4.2, Chrome 56.0.2924.87 and 57.0.2987.110

I have been doing some profiling in Chrome of the render performance of some of our components and noticed a couple of deoptimizations in React.

screen shot 2017-03-17 at 9 21 37 am
screen shot 2017-03-17 at 9 22 00 am

The message Chrome reports is "Not optimized: optimized too many times", which apparently happens when V8 optimizes a function but when running it found a reason to deopt it, and this happened more than max_opt_count setting (currently set to 10). More information: GoogleChrome/devtools-docs#53 (comment)

Clicking through the profile, it looks like most of the time is perhaps spent on this line, but I'm not sure if that is a clue as to what the deopt might be in this case.

nextProp = this._previousStyleCopy = _assign({}, nextProp);

screen shot 2017-03-17 at 9 26 57 am

It might be worth spending some time digging into this, but I'm not sure if it will still be relevant after React 16.

@gaearon
Copy link
Collaborator

gaearon commented Mar 17, 2017

I don't think this line exists in React Fiber implementation.

Please feel free to experiment with react@next and react-dom@next and file any issues about them though!

@gaearon gaearon closed this as completed Mar 17, 2017
@aweary
Copy link
Contributor

aweary commented Mar 17, 2017

For reference, that line was removed in 447e0a1, but time spent is not a direct indicator of why a function might deopt, so a profile of react-dom@next would be a great idea!

@thysultan
Copy link

thysultan commented Apr 3, 2017

Just came across this deopt as well, it's deopting because a named argument nextProps is being reassigned.

function (lastProps, nextProps, transation) {
   // ...
    nextProp = this._previousStyleCopy = _assign({}, nextProp);

So it may affect fiber if there are similar patterns there.

@gaearon
Copy link
Collaborator

gaearon commented Apr 4, 2017

I think you might be confusing nextProps and nextProp. Also AFAIK it was deopting because lastProps is sometimes null but we're using a for in loop. In either case, I don't think fixing that gave any meaningful effect in my testing a while ago, and Fiber code is structured a bit differently. You're welcome to check it for deopts though (we have alphas published).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants