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

Record "actual" times for all Fibers within a Profiler tree (alt) #12910

Merged
merged 8 commits into from May 25, 2018

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented May 25, 2018

Alternate implementation of PR #12891

Trades the ability to accumulate "actual" time across renders of different priorities in exchange for simplifying how/when we need to reset times. Theoretically, this approach is also more compatible with future resuming behavior.

Brian Vaughn added 7 commits May 25, 2018 09:48
A global 'batch id' is updated during the commit phase. Each (profiled) Fiber has a local batch id.

When the Profiler timer is started- the local id is compared to the global one in order to determine whether to reset or accumulate duration.

This has the downside of adding an additional field to fibers (for profiling builds only) but the upside of containing the writes to 'render' phase and allowing the DevTools commit hook to run before durations have been cleared out (so it can read them for profiling mode).
This change gives up on accumulating time across renders of different priority, but in exchange- simplifies how the commit phase (reset) code works, and perhaps also makes the profiling code more compatible with future resuming behavior
// This prevents time from endlessly accumulating in new commits.
workInProgress.actualDuration = 0;
workInProgress.actualStartTime = 0;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can be moved to the else block above. For the initial clone, these are already 0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, although I'll have to break this up so that selfBaseTime and treeBaseTime get copied in both cases. No problem though!

@pull-bot
Copy link

Details of bundled changes.

Comparing: 76e0707...edc7a5d

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.development.js +0.1% +0.1% 621.6 KB 622.03 KB 144.79 KB 144.87 KB UMD_DEV
react-dom.development.js +0.1% +0.1% 605.96 KB 606.4 KB 140.78 KB 140.86 KB NODE_DEV
ReactDOM-dev.js +0.1% +0.1% 628.26 KB 628.68 KB 143.54 KB 143.62 KB FB_WWW_DEV
ReactDOM-prod.js -0.1% -0.0% 273.74 KB 273.59 KB 51.53 KB 51.52 KB FB_WWW_PROD

react-art

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-art.development.js +0.1% +0.1% 403.87 KB 404.31 KB 90.23 KB 90.31 KB UMD_DEV
react-art.development.js +0.1% +0.1% 329.73 KB 330.16 KB 71.34 KB 71.41 KB NODE_DEV
ReactART-dev.js +0.1% +0.1% 335.25 KB 335.68 KB 70.72 KB 70.79 KB FB_WWW_DEV
ReactART-prod.js -0.1% -0.1% 146.84 KB 146.7 KB 25.35 KB 25.32 KB FB_WWW_PROD

react-test-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-test-renderer.development.js +0.1% +0.1% 339.29 KB 339.72 KB 73.33 KB 73.41 KB UMD_DEV
react-test-renderer.development.js +0.1% +0.1% 330.12 KB 330.55 KB 70.59 KB 70.67 KB NODE_DEV
ReactTestRenderer-dev.js +0.1% +0.1% 335.96 KB 336.39 KB 70.17 KB 70.25 KB FB_WWW_DEV

react-reconciler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-reconciler.development.js +0.1% +0.1% 323.8 KB 324.24 KB 68.58 KB 68.67 KB NODE_DEV
react-reconciler-persistent.development.js +0.1% +0.1% 322.36 KB 322.79 KB 67.98 KB 68.06 KB NODE_DEV

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactNativeRenderer-dev.js +0.1% +0.1% 457.44 KB 457.86 KB 100.02 KB 100.09 KB RN_FB_DEV
ReactNativeRenderer-prod.js -0.1% -0.1% 205.83 KB 205.69 KB 35.96 KB 35.94 KB RN_FB_PROD
ReactNativeRenderer-dev.js +0.1% +0.1% 457.1 KB 457.53 KB 99.95 KB 100.03 KB RN_OSS_DEV
ReactFabric-dev.js +0.1% +0.1% 448.35 KB 448.78 KB 97.77 KB 97.85 KB RN_FB_DEV
ReactFabric-dev.js +0.1% +0.1% 448.39 KB 448.82 KB 97.79 KB 97.87 KB RN_OSS_DEV

Generated by 🚫 dangerJS

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

Successfully merging this pull request may close these issues.

None yet

4 participants