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

Add ReactVersion to SchedulingProfiler render scheduled marks #36

Closed
wants to merge 1 commit into from

Conversation

taneliang
Copy link
Member

Summary

These SchedulingProfiler marks are currently being used by our experimental concurrent mode profiler tool.

Context: Brian at #5 (comment):

Since we are actively changing the "lanes" implementation and what the semantic value of a given lane is (e.g. facebook#19287) it would be helpful for us to encode some info in our profile that lets us match this information up later.

I had been thinking about eventually encoding what the lanes ranges are, so we could display some meaningful label in the profiler, but maybe that would be a hassle and for now- we could get away with just marking the version of React the profiling data corresponds to. This would let us at least manually match the lanes up later if we wanted to dig in deeper.

We only add version strings to the render scheduled marks as these marks do not appear often and would thus provide the profiler with information it needs without unnecessarily bloating the profiles.

There is a known limitation to this approach: because renders are not scheduled often (generally only on page load), it is possible to generate profiles that do not contain React version numbers. Ideally, we'd want to log ReactVersion when we detect the start of a profiling session, but as far as we can tell this is currently not possible without integrating with Chrome DevTools.

Resolves #35, and unblocks MLH-Fellowship/scheduling-profiler-prototype#73

Test Plan

yarn test: tests for SchedulingProfiler have been updated. The new render scheduled marks look like this: --schedule-render-1-17.0.0-alpha.0.

@taneliang taneliang force-pushed the eliang/react-version-in-mark branch 2 times, most recently from 7f8f107 to 8a75dc4 Compare August 6, 2020 07:23
@taneliang taneliang force-pushed the eliang/react-version-in-mark branch from 8a75dc4 to e43a127 Compare August 7, 2020 08:11
@taneliang
Copy link
Member Author

PR to upstream opened, superseding this. Closing.

facebook#19553

@taneliang taneliang closed this Aug 10, 2020
taneliang pushed a commit that referenced this pull request Aug 17, 2020
taneliang pushed a commit that referenced this pull request Aug 19, 2020
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

Successfully merging this pull request may close these issues.

Add React version number to profiling marks
2 participants