Skip to content

Remove "Waiting for async callback" User Timing measurement #16379

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

Merged
merged 2 commits into from
Aug 13, 2019
Merged

Remove "Waiting for async callback" User Timing measurement #16379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
73eae39
Remove "Waiting for async callback" User Timing measurement
gaearon Aug 13, 2019
cdcae26
Fix User Timing in PROD mode
gaearon Aug 13, 2019
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
16 changes: 9 additions & 7 deletions packages/react-reconciler/src/ReactFiber.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import warningWithoutStack from 'shared/warningWithoutStack';
import {
enableProfilerTimer,
enableFundamentalAPI,
enableUserTimingAPI,
} from 'shared/ReactFeatureFlags';
import {NoEffect, Placement} from 'shared/ReactSideEffectTags';
import {ConcurrentRoot, BatchedRoot} from 'shared/ReactRootTags';
Expand Down Expand Up @@ -245,11 +246,7 @@ export type Fiber = {|
_debugHookTypes?: Array<HookType> | null,
|};

let debugCounter;

if (__DEV__) {
debugCounter = 1;
}
let debugCounter = 1;

function FiberNode(
tag: WorkTag,
Expand Down Expand Up @@ -319,11 +316,16 @@ function FiberNode(
this.treeBaseDuration = 0;
}

if (__DEV__) {
// This is normally DEV-only except www when it adds listeners.
// TODO: remove the User Timing integration in favor of Root Events.
if (enableUserTimingAPI) {
this._debugID = debugCounter++;
this._debugIsCurrentlyTiming = false;
}

if (__DEV__) {
this._debugSource = null;
this._debugOwner = null;
this._debugIsCurrentlyTiming = false;
this._debugNeedsRemount = false;
this._debugHookTypes = null;
if (!hasBadMapPolyfill && typeof Object.preventExtensions === 'function') {
Expand Down
20 changes: 0 additions & 20 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,6 @@ import {
import {
recordEffect,
recordScheduleUpdate,
startRequestCallbackTimer,
stopRequestCallbackTimer,
startWorkTimer,
stopWorkTimer,
stopFailedWorkTimer,
Expand Down Expand Up @@ -546,16 +544,6 @@ function scheduleCallbackForRoot(
),
options,
);
if (
enableUserTimingAPI &&
expirationTime !== Sync &&
(executionContext & (RenderContext | CommitContext)) === NoContext
) {
// Scheduled an async callback, and we're not already working. Add an
// entry to the flamegraph that shows we're waiting for a callback
// to fire.
startRequestCallbackTimer();
}
}
}

Expand Down Expand Up @@ -816,11 +804,6 @@ function renderRoot(
'Should not already be working.',
);

if (enableUserTimingAPI && expirationTime !== Sync) {
const didExpire = isSync;
stopRequestCallbackTimer(didExpire);
}

if (root.firstPendingTime < expirationTime) {
// If there's no work left at this expiration time, exit immediately. This
// happens when multiple callbacks are scheduled for a single root, but an
Expand Down Expand Up @@ -964,9 +947,6 @@ function renderRoot(
if (workInProgress !== null) {
// There's still work left over. Return a continuation.
stopInterruptedWorkLoopTimer();
if (expirationTime !== Sync) {
startRequestCallbackTimer();
}
return renderRoot.bind(null, root, expirationTime);
}
}
Expand Down
9 changes: 9 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactIncrementalPerf-test.internal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,15 @@ describe('ReactDebugFiberPerf', () => {
// We don't use the overload with three arguments.
measure(label, markName) {
if (markName !== activeMeasure.markName) {
// Fail the test.
console.error(
'Unexpected measure() call: "%s". Active mark is "%s".',
markName,
activeMeasure.markName,
);
// This exception will be caught and ignored
// because in the real implementation, we don't want
// to spam the console due to a React bug.
throw new Error('Unexpected measure() call.');
}
// Step one level up
Expand Down
Loading