-
Notifications
You must be signed in to change notification settings - Fork 47.6k
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
[Fiber] Set profiler values to doubles #30942
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Comparing: bac33d1...2dff09c Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
I've also noticed that the effects in the profiling builds are like 2x as slow or more because of the time spent in the
I assumed that this is because we're searching back up the tree for the Profiler component to store the duration on, but would this also depot, because react/packages/react-reconciler/src/ReactFiber.js Lines 827 to 830 in 8d68da3
|
The stateNode is always created as a pointer field in createFiber. It’s just updated later. So that should be fine. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix!
Build diff view doesn't load (500) but your description makes sense.
At some point this trick was added to initialize the value first to NaN and then replace them with zeros and negative ones. This is to address the issue noted in #14365 where these hidden classes can be initialized to SMIs at first and then deopt when we realize they're actually doubles. However, this fix has been long broken and has deopted the profiling build for years because closure compiler optimizes out the first write. I'm not sure because I haven't A/B-tested this in the JIT yet but I think we can use negative zero and -1.1 as the initial values instead since they're not simple integers. Negative zero `===` zero (but not Object.is) so is the same as far as our code is concerned. The negative value is just `< 0` comparisons. DiffTrain build for commit 94e4aca.
At some point this trick was added to initialize the value first to NaN and then replace them with zeros and negative ones. This is to address the issue noted in #14365 where these hidden classes can be initialized to SMIs at first and then deopt when we realize they're actually doubles. However, this fix has been long broken and has deopted the profiling build for years because closure compiler optimizes out the first write. I'm not sure because I haven't A/B-tested this in the JIT yet but I think we can use negative zero and -1.1 as the initial values instead since they're not simple integers. Negative zero `===` zero (but not Object.is) so is the same as far as our code is concerned. The negative value is just `< 0` comparisons. DiffTrain build for [94e4aca](94e4aca)
**breaking change for canary users: Bumps peer dependency of React from `19.0.0-rc-206df66e-20240912` to `19.0.0-rc-a99d8e8d-20240916`** [diff facebook/react@206df66e...a99d8e8d](facebook/react@206df66...a99d8e8) <details> <summary>React upstream changes</summary> - facebook/react#30977 - facebook/react#30971 - facebook/react#30922 - facebook/react#30917 - facebook/react#30902 - facebook/react#30912 - facebook/react#30970 - facebook/react#30969 - facebook/react#30967 - facebook/react#30966 - facebook/react#30960 - facebook/react#30968 - facebook/react#30961 - facebook/react#28255 - facebook/react#30957 - facebook/react#30958 - facebook/react#30959 - facebook/react#30951 - facebook/react#30954 - facebook/react#30920 - facebook/react#30942 </details>
At some point this trick was added to initialize the value first to NaN and then replace them with zeros and negative ones.
This is to address the issue noted in #14365 where these hidden classes can be initialized to SMIs at first and then deopt when we realize they're actually doubles.
However, this fix has been long broken and has deopted the profiling build for years because closure compiler optimizes out the first write.
I'm not sure because I haven't A/B-tested this in the JIT yet but I think we can use negative zero and -1.1 as the initial values instead since they're not simple integers. Negative zero
===
zero (but not Object.is) so is the same as far as our code is concerned. The negative value is just< 0
comparisons.