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

Log User Timing API entries for Profiler nodes in the production+prof… #15260

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
6 participants
@bvaughn
Copy link
Contributor

commented Mar 29, 2019

Resolves issue #15255 cc @bgirard

React uses the User Timing API to mark and measure time spent working on various things for development builds. This can be a useful source of information when it comes to cross-referencing browser performance traces. We avoid doing this in production mode though, due to the performance overhead of using the API.

I believe that we could make an exception for our profiling bundles and log measurements for Profiler nodes only. This should be a lot less heavyweight than the full development measurements, while still providing a potentially useful signal in the trace logs.

Here's an initial pass at this. This change has a minimal impact on the development bundle behavior- (including the Profiler id as part of the display name)- which seemed safe enough. I also verified that none of the timer related code leaks into the production bundle.

Development bundle

Screen Shot 2019-03-29 at 1 25 51 PM

Profiling bundle

Screen Shot 2019-03-29 at 1 30 42 PM

@sizebot

This comment has been minimized.

Copy link

commented Mar 29, 2019

ReactDOM: size: 0.0%, gzip: 0.0%

Details of bundled changes.

Comparing: 5ef0d1d...f4a8940

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.development.js +0.1% +0.1% 813.79 KB 814.24 KB 184.27 KB 184.37 KB UMD_DEV
react-dom.production.min.js 0.0% 0.0% 105.12 KB 105.12 KB 33.97 KB 33.98 KB UMD_PROD
react-dom.profiling.min.js +1.6% +1.7% 108.1 KB 109.83 KB 34.63 KB 35.21 KB UMD_PROFILING
react-dom.development.js +0.1% 0.0% 808.18 KB 808.63 KB 182.67 KB 182.76 KB NODE_DEV
react-dom.production.min.js 0.0% 0.0% 105.1 KB 105.1 KB 33.47 KB 33.48 KB NODE_PROD
react-dom.profiling.min.js +1.6% +1.6% 108.2 KB 109.94 KB 34.08 KB 34.63 KB NODE_PROFILING
ReactDOM-dev.js +0.1% 0.0% 832.93 KB 833.39 KB 184.3 KB 184.39 KB FB_WWW_DEV
ReactDOM-prod.js 0.0% 🔺+0.1% 331.38 KB 331.53 KB 61.04 KB 61.08 KB FB_WWW_PROD
ReactDOM-profiling.js 0.0% +0.1% 337.76 KB 337.79 KB 62.39 KB 62.45 KB FB_WWW_PROFILING
react-dom-unstable-fire.development.js +0.1% +0.1% 814.13 KB 814.59 KB 184.42 KB 184.51 KB UMD_DEV
react-dom-unstable-fire.production.min.js 0.0% 0.0% 105.14 KB 105.14 KB 33.98 KB 33.99 KB UMD_PROD
react-dom-unstable-fire.profiling.min.js +1.6% +1.7% 108.12 KB 109.85 KB 34.64 KB 35.22 KB UMD_PROFILING
react-dom-unstable-fire.development.js +0.1% +0.1% 808.52 KB 808.97 KB 182.81 KB 182.9 KB NODE_DEV
react-dom-unstable-fire.production.min.js 0.0% 0.0% 105.11 KB 105.11 KB 33.48 KB 33.49 KB NODE_PROD
react-dom-unstable-fire.profiling.min.js +1.6% +1.6% 108.22 KB 109.95 KB 34.09 KB 34.64 KB NODE_PROFILING
ReactFire-dev.js +0.1% 0.0% 832.14 KB 832.6 KB 184.23 KB 184.32 KB FB_WWW_DEV
ReactFire-prod.js 0.0% 0.0% 320.04 KB 320.04 KB 58.71 KB 58.71 KB FB_WWW_PROD
ReactFire-profiling.js +1.2% +1.3% 326.37 KB 330.34 KB 60.06 KB 60.87 KB FB_WWW_PROFILING
react-dom-test-utils.development.js 0.0% 0.0% 47.49 KB 47.49 KB 12.98 KB 12.98 KB UMD_DEV
react-dom-test-utils.production.min.js 0.0% 🔺+0.1% 9.95 KB 9.95 KB 3.66 KB 3.66 KB UMD_PROD
react-dom-test-utils.development.js 0.0% 0.0% 47.21 KB 47.21 KB 12.91 KB 12.91 KB NODE_DEV
react-dom-test-utils.production.min.js 0.0% 0.0% 9.73 KB 9.73 KB 3.59 KB 3.59 KB NODE_PROD
react-dom-unstable-native-dependencies.development.js 0.0% 0.0% 60.74 KB 60.74 KB 15.84 KB 15.84 KB UMD_DEV
react-dom-unstable-native-dependencies.production.min.js 0.0% 🔺+0.1% 10.69 KB 10.69 KB 3.67 KB 3.67 KB UMD_PROD
react-dom-unstable-native-dependencies.development.js 0.0% 0.0% 60.41 KB 60.41 KB 15.71 KB 15.72 KB NODE_DEV
react-dom-unstable-native-dependencies.production.min.js 0.0% 🔺+0.1% 10.42 KB 10.42 KB 3.56 KB 3.57 KB NODE_PROD
react-dom-server.browser.development.js 0.0% 0.0% 134.78 KB 134.78 KB 35.51 KB 35.51 KB UMD_DEV
react-dom-server.browser.production.min.js 0.0% 0.0% 19.05 KB 19.05 KB 7.18 KB 7.18 KB UMD_PROD
react-dom-server.browser.development.js 0.0% 0.0% 130.91 KB 130.91 KB 34.57 KB 34.57 KB NODE_DEV
react-dom-server.browser.production.min.js 0.0% 0.0% 18.98 KB 18.98 KB 7.17 KB 7.17 KB NODE_PROD
ReactDOMServer-dev.js 0.0% 0.0% 133.22 KB 133.22 KB 34.32 KB 34.32 KB FB_WWW_DEV
ReactDOMServer-prod.js 0.0% -0.0% 46.19 KB 46.19 KB 10.63 KB 10.62 KB FB_WWW_PROD
react-dom-server.node.development.js 0.0% 0.0% 132.86 KB 132.86 KB 35.12 KB 35.12 KB NODE_DEV
react-dom-server.node.production.min.js 0.0% 0.0% 19.84 KB 19.84 KB 7.48 KB 7.48 KB NODE_PROD
react-dom-unstable-fizz.browser.development.js 0.0% +0.1% 3.66 KB 3.66 KB 1.45 KB 1.45 KB UMD_DEV
react-dom-unstable-fizz.browser.development.js 0.0% +0.1% 3.49 KB 3.49 KB 1.41 KB 1.41 KB NODE_DEV
react-dom-unstable-fizz.browser.production.min.js 0.0% 🔺+0.2% 1.05 KB 1.05 KB 636 B 637 B NODE_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% 567 KB 567.45 KB 122.43 KB 122.54 KB UMD_DEV
react-art.production.min.js 0.0% 0.0% 96.78 KB 96.78 KB 29.68 KB 29.68 KB UMD_PROD
react-art.development.js +0.1% +0.1% 497.91 KB 498.36 KB 105.04 KB 105.17 KB NODE_DEV
react-art.production.min.js 0.0% 0.0% 61.78 KB 61.78 KB 18.91 KB 18.91 KB NODE_PROD
ReactART-dev.js +0.1% +0.1% 507.84 KB 508.29 KB 104.34 KB 104.47 KB FB_WWW_DEV
ReactART-prod.js 0.0% 0.0% 196.63 KB 196.63 KB 33.3 KB 33.3 KB FB_WWW_PROD

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactNativeRenderer-dev.js +0.1% +0.1% 629 KB 629.45 KB 134.15 KB 134.25 KB RN_FB_DEV
ReactNativeRenderer-prod.js 0.0% 0.0% 246.02 KB 246.02 KB 43.02 KB 43.02 KB RN_FB_PROD
ReactNativeRenderer-profiling.js +1.6% +1.8% 252.26 KB 256.24 KB 44.37 KB 45.17 KB RN_FB_PROFILING
ReactNativeRenderer-dev.js +0.1% +0.1% 628.92 KB 629.37 KB 134.12 KB 134.22 KB RN_OSS_DEV
ReactNativeRenderer-prod.js 0.0% 0.0% 246.03 KB 246.03 KB 43.02 KB 43.02 KB RN_OSS_PROD
ReactNativeRenderer-profiling.js +1.6% +1.8% 252.28 KB 256.26 KB 44.37 KB 45.17 KB RN_OSS_PROFILING
ReactFabric-dev.js +0.1% +0.1% 617.85 KB 618.3 KB 131.51 KB 131.62 KB RN_FB_DEV
ReactFabric-prod.js 0.0% 0.0% 239.42 KB 239.42 KB 41.76 KB 41.76 KB RN_FB_PROD
ReactFabric-profiling.js +1.6% +1.9% 244.76 KB 248.75 KB 43.08 KB 43.88 KB RN_FB_PROFILING
ReactFabric-dev.js +0.1% +0.1% 617.76 KB 618.21 KB 131.46 KB 131.57 KB RN_OSS_DEV
ReactFabric-prod.js 0.0% 0.0% 239.43 KB 239.43 KB 41.75 KB 41.75 KB RN_OSS_PROD
ReactFabric-profiling.js +1.6% +1.8% 244.77 KB 248.76 KB 43.08 KB 43.87 KB RN_OSS_PROFILING

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% 509.45 KB 509.9 KB 107.2 KB 107.31 KB UMD_DEV
react-test-renderer.production.min.js 0.0% 0.0% 62.77 KB 62.77 KB 19.14 KB 19.14 KB UMD_PROD
react-test-renderer.development.js +0.1% +0.1% 504.97 KB 505.42 KB 106.04 KB 106.16 KB NODE_DEV
react-test-renderer.production.min.js 0.0% 0.0% 62.46 KB 62.46 KB 18.96 KB 18.96 KB NODE_PROD
ReactTestRenderer-dev.js +0.1% +0.1% 516.15 KB 516.6 KB 105.82 KB 105.92 KB FB_WWW_DEV
react-test-renderer-shallow.production.min.js 0.0% 0.0% 11.42 KB 11.42 KB 3.5 KB 3.51 KB UMD_PROD
react-test-renderer-shallow.development.js 0.0% 0.0% 33.2 KB 33.2 KB 8.41 KB 8.42 KB NODE_DEV
react-test-renderer-shallow.production.min.js 0.0% 0.0% 11.61 KB 11.61 KB 3.63 KB 3.63 KB NODE_PROD

react-reconciler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-reconciler.development.js +0.1% +0.1% 498 KB 498.46 KB 103.88 KB 103.98 KB NODE_DEV
react-reconciler.production.min.js 0.0% 0.0% 62.97 KB 62.97 KB 18.73 KB 18.73 KB NODE_PROD
react-reconciler-persistent.development.js +0.1% +0.1% 495.89 KB 496.35 KB 103.01 KB 103.12 KB NODE_DEV
react-reconciler-persistent.production.min.js 0.0% 0.0% 62.98 KB 62.98 KB 18.74 KB 18.74 KB NODE_PROD
react-reconciler-reflection.development.js 0.0% 0.0% 15.79 KB 15.79 KB 4.91 KB 4.91 KB NODE_DEV
react-reconciler-reflection.production.min.js 0.0% 🔺+0.2% 2.37 KB 2.37 KB 1.07 KB 1.07 KB NODE_PROD

Generated by 🚫 dangerJS

@bvaughn bvaughn force-pushed the bvaughn:profiler-mark-measure branch from b0273c6 to 8a4203e Mar 29, 2019

@bgirard

This comment has been minimized.

Copy link
Contributor

commented Mar 29, 2019

Looks great!

@threepointone
Copy link
Contributor

left a comment

looks good to me!

@@ -123,7 +135,7 @@ const beginFiberMark = (
fiber: Fiber,
phase: MeasurementPhase | null,
): boolean => {
const componentName = getComponentName(fiber.type) || 'Unknown';
let componentName = getFiberName(fiber);

This comment has been minimized.

Copy link
@threepointone

threepointone Apr 1, 2019

Contributor

why did you make this a let?

This comment has been minimized.

Copy link
@bvaughn

bvaughn Apr 1, 2019

Author Contributor

🤷‍♂️ Oversight. Will change.

@bvaughn bvaughn force-pushed the bvaughn:profiler-mark-measure branch from 8a4203e to f4a8940 Apr 1, 2019

@sebmarkbage

This comment has been minimized.

Copy link
Member

commented Apr 1, 2019

What's the contract (expectation of continued support) of this API? Is it only useful if it's reliable and can be counted on? If it heavily loses its usefulness when it's not reliable and we can't keep it reliable, then it might be more a source a noise than not.

We have a plan to start reconciling deeper in a tree instead of starting from the root, especially for pings and "retries" where we know it can't affect surroundings. In that scenario, we could skip past the Profiler nodes. We could still patch up profiling data (the inclusive time) by backtracking after the fact, but that wouldn't show in the timeline as wrapped. This optimization would also affect the DEV mode but since we're including everything there it might be a bit more obvious that we're cutting out a few levels.

@bvaughn

This comment has been minimized.

Copy link
Contributor Author

commented Apr 1, 2019

What's the contract of this API?

This is a good question. To be honest, I didn't intend to suggest a contract here really. It just seemed like this would offer some potentially useful additional data points for some of our real world users.

If it heavily loses its usefulness when it's not reliable and we can't keep it reliable, then it might be more a source a noise than not.

Fair enough.

It seems like this is pain point / a gap for users currently though. If you'd like, I think I could put together an alternate proposal that's a bit more robust to the deep re-rendering concern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.