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

Update to new ReactPerf #7283

Closed
wants to merge 1 commit into from
Closed

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Apr 28, 2016

This is a work in progress on removing the dependency on the old ReactPerf and replacing it with the new one. Related PRs in React: facebook/react#6647, facebook/react#6046.

@ghost
Copy link

ghost commented Apr 28, 2016

By analyzing the blame information on this pull request, we identified @sebmarkbage and @tadeuzagallo to be potential reviewers.

@ghost ghost added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Apr 28, 2016
@@ -11,7 +11,7 @@
*/
'use strict';

var ReactDefaultPerf = require('ReactDefaultPerf');
var ReactPerfAnalysis = require('ReactPerfAnalysis');

Choose a reason for hiding this comment

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

ReactPerfAnalysis Required module not found

@ghost
Copy link

ghost commented Apr 29, 2016

@gaearon updated the pull request.

}
function ReactComponentTreeDevtool() {
if (!_ReactComponentTreeDevtool) {
_ReactComponentTreeDevtool = require('ReactComponentTreeDevtool');

Choose a reason for hiding this comment

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

ReactComponentTreeDevtool Required module not found

@ghost
Copy link

ghost commented Apr 29, 2016

@gaearon updated the pull request.

}
function ReactComponentTreeDevtool() {
if (!_ReactComponentTreeDevtool) {
_ReactComponentTreeDevtool = require('ReactComponentTreeDevtool');

Choose a reason for hiding this comment

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

ReactComponentTreeDevtool Required module not found

@gaearon gaearon changed the title (WIP) Update to new ReactPerf Update to new ReactPerf Apr 29, 2016
@facebook-github-bot
Copy link
Contributor

@gaearon updated the pull request.

}
function ReactComponentTreeDevtool() {
if (!_ReactComponentTreeDevtool) {
_ReactComponentTreeDevtool = require('ReactComponentTreeDevtool');

Choose a reason for hiding this comment

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

ReactComponentTreeDevtool Required module not found

Choose a reason for hiding this comment

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

ReactComponentTreeDevtool Required module not found

@gaearon
Copy link
Collaborator Author

gaearon commented Apr 29, 2016

This is ready for review.
It depends on facebook/react#6647.

@sebmarkbage

@satya164
Copy link
Contributor

@gaearon Tests seem to be broken!

@gaearon
Copy link
Collaborator Author

gaearon commented Apr 29, 2016

Sorry, I should have been clear.
It depends on a version of React that hasn’t been released yet.
I won’t be available the next two weeks so I’m leaving this for the team to merge during the next RN sync.

@satya164
Copy link
Contributor

@gaearon Okay then. I'm assigning this to @sebmarkbage as he usually does the sync.

@gaearon
Copy link
Collaborator Author

gaearon commented Apr 30, 2016

Test plan:

  1. Launch any app
  2. Cmd D, Start Systrace
  3. Mount, update, unmount some views
  4. Cmd D, Stop Systrace
  5. Drop generated JSON on about:tracing and check it contains reconciler and composite events

For the other change,

  1. Add RCTRenderingPerf toggle() and start() calls on init
  2. Add stop() call in a timeout
  3. Do stuff
  4. Check that it logs tables similar to the tables on the web

@sophiebits sophiebits assigned sophiebits and unassigned sebmarkbage May 10, 2016
@sophiebits
Copy link
Contributor

Landed as part of a95405f, thanks.

@sophiebits sophiebits closed this May 10, 2016
@gaearon gaearon deleted the react-perf branch May 10, 2016 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants