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

Fiber logs timing for HostRoot if logTopLevelRenders enabled #8687

Merged
merged 1 commit into from
Jan 21, 2017

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Jan 4, 2017

Adds calls to console.time and console.timeEnd before beginning work on the top-level (HostRoot) component if ReactFeatureFlags.logTopLevelRenders is enabled.

Couple of concerns / open questions about this approach:

  • Is this equivalent (enough) to the timing information stack logs?
  • Are we concerned about timing in the case of bailouts caused by higher-priority updates? What about work that gets broken up across frames?

Update: This implementation potentially logs a single time entry for multiple roots, since workLoop calls findNextUnitOfWork which searches across roots. That's probably not what we want. Maybe we could only enable this logging when useSyncScheduling is enabled?

@bvaughn bvaughn changed the title Fiber logs timing for HostRoot if logTopLevelRenders flag enabled Fiber logs timing for HostRoot if logTopLevelRenders enabled Jan 4, 2017
@acdlite
Copy link
Collaborator

acdlite commented Jan 4, 2017

@spicyj What is logTopLevelRenders intended to measure? Time to initial render?

@bvaughn
Copy link
Contributor Author

bvaughn commented Jan 4, 2017

I think the intent is to annotate the Chrome Timeline view to make it easier to match frames to React components. Not just initial render but any updates (given that a0f88d2 added logging to ReactUpdates and ReactMount).

@sebmarkbage
Copy link
Collaborator

This is really used as a FB specific feature to measure the overall initial render time when profiling.

@spicyj Does this look like the right thing we want to measure? Basically for sync/task/animation mode it will log the reconciliation time for an update to a root. For deferred work, it'll measure the time of a particular slice. Do we still need this feature for Web Speed?

I wonder if maybe we shouldn't do this for deferred priority work since those can't be measured as a single unit.

Copy link
Collaborator

@sebmarkbage sebmarkbage left a comment

Choose a reason for hiding this comment

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

Let's get this in for parity. I'm not sure if it will always measure something we're interested in but we can revert later.

Initial stab at calling console.time/console.timeEnd for the top level component when ReactFeatureFlags.logTopLevelRenders is enabled
@bvaughn bvaughn merged commit f546505 into facebook:master Jan 21, 2017
@bvaughn bvaughn deleted the top-level-renderer-timer branch January 21, 2017 17:44
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.

4 participants