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.

Sign up for GitHub

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

Jump to bottom

Show React events in the timeline when ReactPerf is active #7549

Merged
merged 2 commits into from
Aug 24, 2016
Merged

Show React events in the timeline when ReactPerf is active #7549

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
57ff4d8
Show React events in the timeline when ReactPerf is active
gaearon Aug 23, 2016
b0a0bf5
Address Chrome issue
gaearon Aug 24, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions src/renderers/shared/ReactDebugTool.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,63 @@ function resumeCurrentLifeCycleTimer() {
currentTimerType = timerType;
}

var lastMarkTimeStamp = null;
var canUsePerformanceMeasure =
typeof performance !== 'undefined' &&
typeof performance.mark === 'function' &&
typeof performance.clearMarks === 'function' &&
typeof performance.measure === 'function' &&
typeof performance.clearMeasures === 'function';

function shouldMark(debugID) {
if (!isProfiling || !canUsePerformanceMeasure) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably overkill but if canUsePerformanceMeasure is not true, I would just do shouldMark = function() {}.

You could use the same technique by clearing shouldMark when you end profiling and bring it back when you start profiling. This way you don't even have to do that test.

I have no idea what are the perf implications of this but since we're talking about super hot functions, it may be worth it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I took a profile and it was something like 0.01% of total time so I didn’t bother.

Copy link

Choose a reason for hiding this comment

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

Is all those checks necessary? Can't we simply check 'mark' in performance or 'measure' in performance and just assume the User Timing API is supported?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is there any problem with doing them? In my experience of using standards, unless you check for everything explicitly, you risk running into bad/incomplete polyfills.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also those checks run only once.

Copy link

Choose a reason for hiding this comment

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

Not sure if its a overkill. But wanted to confirm if there is any weird behaviour with any browsers.

Totally agree on that point 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No weird behavior so far, just wanted to be extra safe.

Copy link

Choose a reason for hiding this comment

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

Cool. Sounds good.

return false;
}
var element = ReactComponentTreeHook.getElement(debugID);
if (element == null || typeof element !== 'object') {
return false;
}
var isHostElement = typeof element.type === 'string';
if (isHostElement) {
return false;
}
return true;
}

function markBegin(debugID, markType) {
if (!shouldMark(debugID)) {
return;
}

var markName = `${debugID}::${markType}`;
lastMarkTimeStamp = performanceNow();
performance.mark(markName);
}

function markEnd(debugID, markType) {
if (!shouldMark(debugID)) {
return;
}

var markName = `${debugID}::${markType}`;
var displayName = ReactComponentTreeHook.getDisplayName(debugID);

// Chrome has an issue of dropping markers recorded too fast:
// https://bugs.chromium.org/p/chromium/issues/detail?id=640652
// To work around this, we will not report very small measurements.
// I determined the magic number by tweaking it back and forth.
// 0.05ms was enough to prevent the issue, but I set it to 0.1ms to be safe.
// When the bug is fixed, we can `measure()` unconditionally if we want to.
var timeStamp = performanceNow();
if (timeStamp - lastMarkTimeStamp > 0.1) {
var measurementName = `${displayName} [${markType}]`;
performance.measure(measurementName, markName);
}

performance.clearMarks(markName);
performance.clearMeasures(measurementName);
}

var ReactDebugTool = {
addHook(hook) {
hooks.push(hook);
Expand Down Expand Up @@ -244,11 +301,13 @@ var ReactDebugTool = {
onBeginLifeCycleTimer(debugID, timerType) {
checkDebugID(debugID);
emitEvent('onBeginLifeCycleTimer', debugID, timerType);
markBegin(debugID, timerType);
Copy link
Contributor

Choose a reason for hiding this comment

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

So good, I was going to ask if you could make this less verbose and repetitive :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I wrote this PR yesterday while waiting for the taxi so it wasn’t really clean 😄

beginLifeCycleTimer(debugID, timerType);
},
onEndLifeCycleTimer(debugID, timerType) {
checkDebugID(debugID);
endLifeCycleTimer(debugID, timerType);
markEnd(debugID, timerType);
emitEvent('onEndLifeCycleTimer', debugID, timerType);
},
onError(debugID) {
Expand Down Expand Up @@ -279,25 +338,31 @@ var ReactDebugTool = {
checkDebugID(debugID);
checkDebugID(parentDebugID, true);
emitEvent('onBeforeMountComponent', debugID, element, parentDebugID);
markBegin(debugID, 'mount');
},
onMountComponent(debugID) {
checkDebugID(debugID);
markEnd(debugID, 'mount');
emitEvent('onMountComponent', debugID);
},
onBeforeUpdateComponent(debugID, element) {
checkDebugID(debugID);
emitEvent('onBeforeUpdateComponent', debugID, element);
markBegin(debugID, 'update');
},
onUpdateComponent(debugID) {
checkDebugID(debugID);
markEnd(debugID, 'update');
emitEvent('onUpdateComponent', debugID);
},
onBeforeUnmountComponent(debugID) {
checkDebugID(debugID);
emitEvent('onBeforeUnmountComponent', debugID);
markBegin(debugID, 'unmount');
},
onUnmountComponent(debugID) {
checkDebugID(debugID);
markEnd(debugID, 'unmount');
emitEvent('onUnmountComponent', debugID);
},
onTestEvent() {
Expand Down