Skip to content

Commit

Permalink
Profiler: Include ref callbacks in onCommit duration (#20060)
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Vaughn authored Oct 20, 2020
1 parent c59c3df commit d1bb4d8
Show file tree
Hide file tree
Showing 3 changed files with 163 additions and 8 deletions.
58 changes: 54 additions & 4 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,14 +194,38 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber) {
if (ref !== null) {
if (typeof ref === 'function') {
if (__DEV__) {
invokeGuardedCallback(null, ref, null, null);
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
current.mode & ProfileMode
) {
startLayoutEffectTimer();
invokeGuardedCallback(null, ref, null, null);
recordLayoutEffectDuration(current);
} else {
invokeGuardedCallback(null, ref, null, null);
}

if (hasCaughtError()) {
const refError = clearCaughtError();
captureCommitPhaseError(current, nearestMountedAncestor, refError);
}
} else {
try {
ref(null);
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
current.mode & ProfileMode
) {
try {
startLayoutEffectTimer();
ref(null);
} finally {
recordLayoutEffectDuration(current);
}
} else {
ref(null);
}
} catch (refError) {
captureCommitPhaseError(current, nearestMountedAncestor, refError);
}
Expand Down Expand Up @@ -965,7 +989,20 @@ function commitAttachRef(finishedWork: Fiber) {
instanceToUse = instance;
}
if (typeof ref === 'function') {
ref(instanceToUse);
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
finishedWork.mode & ProfileMode
) {
try {
startLayoutEffectTimer();
ref(instanceToUse);
} finally {
recordLayoutEffectDuration(finishedWork);
}
} else {
ref(instanceToUse);
}
} else {
if (__DEV__) {
if (!ref.hasOwnProperty('current')) {
Expand All @@ -986,7 +1023,20 @@ function commitDetachRef(current: Fiber) {
const currentRef = current.ref;
if (currentRef !== null) {
if (typeof currentRef === 'function') {
currentRef(null);
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
current.mode & ProfileMode
) {
try {
startLayoutEffectTimer();
currentRef(null);
} finally {
recordLayoutEffectDuration(current);
}
} else {
currentRef(null);
}
} else {
currentRef.current = null;
}
Expand Down
58 changes: 54 additions & 4 deletions packages/react-reconciler/src/ReactFiberCommitWork.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,14 +180,38 @@ function safelyDetachRef(current: Fiber) {
if (ref !== null) {
if (typeof ref === 'function') {
if (__DEV__) {
invokeGuardedCallback(null, ref, null, null);
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
current.mode & ProfileMode
) {
startLayoutEffectTimer();
invokeGuardedCallback(null, ref, null, null);
recordLayoutEffectDuration(current);
} else {
invokeGuardedCallback(null, ref, null, null);
}

if (hasCaughtError()) {
const refError = clearCaughtError();
captureCommitPhaseError(current, refError);
}
} else {
try {
ref(null);
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
current.mode & ProfileMode
) {
try {
startLayoutEffectTimer();
ref(null);
} finally {
recordLayoutEffectDuration(current);
}
} else {
ref(null);
}
} catch (refError) {
captureCommitPhaseError(current, refError);
}
Expand Down Expand Up @@ -832,7 +856,20 @@ function commitAttachRef(finishedWork: Fiber) {
instanceToUse = instance;
}
if (typeof ref === 'function') {
ref(instanceToUse);
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
finishedWork.mode & ProfileMode
) {
try {
startLayoutEffectTimer();
ref(instanceToUse);
} finally {
recordLayoutEffectDuration(finishedWork);
}
} else {
ref(instanceToUse);
}
} else {
if (__DEV__) {
if (!ref.hasOwnProperty('current')) {
Expand All @@ -853,7 +890,20 @@ function commitDetachRef(current: Fiber) {
const currentRef = current.ref;
if (currentRef !== null) {
if (typeof currentRef === 'function') {
currentRef(null);
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
current.mode & ProfileMode
) {
try {
startLayoutEffectTimer();
currentRef(null);
} finally {
recordLayoutEffectDuration(current);
}
} else {
currentRef(null);
}
} else {
currentRef.current = null;
}
Expand Down
55 changes: 55 additions & 0 deletions packages/react/src/__tests__/ReactProfiler-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -1491,6 +1491,61 @@ describe('Profiler', () => {
expect(call[4]).toEqual(enableSchedulerTracing ? new Set() : undefined); // interaction events
});

it('should include time spent in ref callbacks', () => {
const callback = jest.fn();

const refSetter = ref => {
if (ref !== null) {
Scheduler.unstable_advanceTime(10);
} else {
Scheduler.unstable_advanceTime(100);
}
};

class ClassComponent extends React.Component {
render() {
return null;
}
}

const Component = () => {
Scheduler.unstable_advanceTime(1000);
return <ClassComponent ref={refSetter} />;
};

Scheduler.unstable_advanceTime(1);

const renderer = ReactTestRenderer.create(
<React.Profiler id="root" onCommit={callback}>
<Component />
</React.Profiler>,
);

expect(callback).toHaveBeenCalledTimes(1);

let call = callback.mock.calls[0];

expect(call).toHaveLength(enableSchedulerTracing ? 5 : 4);
expect(call[0]).toBe('root');
expect(call[1]).toBe('mount');
expect(call[2]).toBe(10); // durations
expect(call[3]).toBe(1001); // commit start time (before mutations or effects)

callback.mockClear();

renderer.update(<React.Profiler id="root" onCommit={callback} />);

expect(callback).toHaveBeenCalledTimes(1);

call = callback.mock.calls[0];

expect(call).toHaveLength(enableSchedulerTracing ? 5 : 4);
expect(call[0]).toBe('root');
expect(call[1]).toBe('update');
expect(call[2]).toBe(100); // durations
expect(call[3]).toBe(1011); // commit start time (before mutations or effects)
});

it('should bubble time spent in layout effects to higher profilers', () => {
const callback = jest.fn();

Expand Down

0 comments on commit d1bb4d8

Please sign in to comment.