Skip to content

Commit d1bb4d8

Browse files
author
Brian Vaughn
authored
Profiler: Include ref callbacks in onCommit duration (#20060)
1 parent c59c3df commit d1bb4d8

File tree

3 files changed

+163
-8
lines changed

3 files changed

+163
-8
lines changed

packages/react-reconciler/src/ReactFiberCommitWork.new.js

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,14 +194,38 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber) {
194194
if (ref !== null) {
195195
if (typeof ref === 'function') {
196196
if (__DEV__) {
197-
invokeGuardedCallback(null, ref, null, null);
197+
if (
198+
enableProfilerTimer &&
199+
enableProfilerCommitHooks &&
200+
current.mode & ProfileMode
201+
) {
202+
startLayoutEffectTimer();
203+
invokeGuardedCallback(null, ref, null, null);
204+
recordLayoutEffectDuration(current);
205+
} else {
206+
invokeGuardedCallback(null, ref, null, null);
207+
}
208+
198209
if (hasCaughtError()) {
199210
const refError = clearCaughtError();
200211
captureCommitPhaseError(current, nearestMountedAncestor, refError);
201212
}
202213
} else {
203214
try {
204-
ref(null);
215+
if (
216+
enableProfilerTimer &&
217+
enableProfilerCommitHooks &&
218+
current.mode & ProfileMode
219+
) {
220+
try {
221+
startLayoutEffectTimer();
222+
ref(null);
223+
} finally {
224+
recordLayoutEffectDuration(current);
225+
}
226+
} else {
227+
ref(null);
228+
}
205229
} catch (refError) {
206230
captureCommitPhaseError(current, nearestMountedAncestor, refError);
207231
}
@@ -965,7 +989,20 @@ function commitAttachRef(finishedWork: Fiber) {
965989
instanceToUse = instance;
966990
}
967991
if (typeof ref === 'function') {
968-
ref(instanceToUse);
992+
if (
993+
enableProfilerTimer &&
994+
enableProfilerCommitHooks &&
995+
finishedWork.mode & ProfileMode
996+
) {
997+
try {
998+
startLayoutEffectTimer();
999+
ref(instanceToUse);
1000+
} finally {
1001+
recordLayoutEffectDuration(finishedWork);
1002+
}
1003+
} else {
1004+
ref(instanceToUse);
1005+
}
9691006
} else {
9701007
if (__DEV__) {
9711008
if (!ref.hasOwnProperty('current')) {
@@ -986,7 +1023,20 @@ function commitDetachRef(current: Fiber) {
9861023
const currentRef = current.ref;
9871024
if (currentRef !== null) {
9881025
if (typeof currentRef === 'function') {
989-
currentRef(null);
1026+
if (
1027+
enableProfilerTimer &&
1028+
enableProfilerCommitHooks &&
1029+
current.mode & ProfileMode
1030+
) {
1031+
try {
1032+
startLayoutEffectTimer();
1033+
currentRef(null);
1034+
} finally {
1035+
recordLayoutEffectDuration(current);
1036+
}
1037+
} else {
1038+
currentRef(null);
1039+
}
9901040
} else {
9911041
currentRef.current = null;
9921042
}

packages/react-reconciler/src/ReactFiberCommitWork.old.js

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -180,14 +180,38 @@ function safelyDetachRef(current: Fiber) {
180180
if (ref !== null) {
181181
if (typeof ref === 'function') {
182182
if (__DEV__) {
183-
invokeGuardedCallback(null, ref, null, null);
183+
if (
184+
enableProfilerTimer &&
185+
enableProfilerCommitHooks &&
186+
current.mode & ProfileMode
187+
) {
188+
startLayoutEffectTimer();
189+
invokeGuardedCallback(null, ref, null, null);
190+
recordLayoutEffectDuration(current);
191+
} else {
192+
invokeGuardedCallback(null, ref, null, null);
193+
}
194+
184195
if (hasCaughtError()) {
185196
const refError = clearCaughtError();
186197
captureCommitPhaseError(current, refError);
187198
}
188199
} else {
189200
try {
190-
ref(null);
201+
if (
202+
enableProfilerTimer &&
203+
enableProfilerCommitHooks &&
204+
current.mode & ProfileMode
205+
) {
206+
try {
207+
startLayoutEffectTimer();
208+
ref(null);
209+
} finally {
210+
recordLayoutEffectDuration(current);
211+
}
212+
} else {
213+
ref(null);
214+
}
191215
} catch (refError) {
192216
captureCommitPhaseError(current, refError);
193217
}
@@ -832,7 +856,20 @@ function commitAttachRef(finishedWork: Fiber) {
832856
instanceToUse = instance;
833857
}
834858
if (typeof ref === 'function') {
835-
ref(instanceToUse);
859+
if (
860+
enableProfilerTimer &&
861+
enableProfilerCommitHooks &&
862+
finishedWork.mode & ProfileMode
863+
) {
864+
try {
865+
startLayoutEffectTimer();
866+
ref(instanceToUse);
867+
} finally {
868+
recordLayoutEffectDuration(finishedWork);
869+
}
870+
} else {
871+
ref(instanceToUse);
872+
}
836873
} else {
837874
if (__DEV__) {
838875
if (!ref.hasOwnProperty('current')) {
@@ -853,7 +890,20 @@ function commitDetachRef(current: Fiber) {
853890
const currentRef = current.ref;
854891
if (currentRef !== null) {
855892
if (typeof currentRef === 'function') {
856-
currentRef(null);
893+
if (
894+
enableProfilerTimer &&
895+
enableProfilerCommitHooks &&
896+
current.mode & ProfileMode
897+
) {
898+
try {
899+
startLayoutEffectTimer();
900+
currentRef(null);
901+
} finally {
902+
recordLayoutEffectDuration(current);
903+
}
904+
} else {
905+
currentRef(null);
906+
}
857907
} else {
858908
currentRef.current = null;
859909
}

packages/react/src/__tests__/ReactProfiler-test.internal.js

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1491,6 +1491,61 @@ describe('Profiler', () => {
14911491
expect(call[4]).toEqual(enableSchedulerTracing ? new Set() : undefined); // interaction events
14921492
});
14931493

1494+
it('should include time spent in ref callbacks', () => {
1495+
const callback = jest.fn();
1496+
1497+
const refSetter = ref => {
1498+
if (ref !== null) {
1499+
Scheduler.unstable_advanceTime(10);
1500+
} else {
1501+
Scheduler.unstable_advanceTime(100);
1502+
}
1503+
};
1504+
1505+
class ClassComponent extends React.Component {
1506+
render() {
1507+
return null;
1508+
}
1509+
}
1510+
1511+
const Component = () => {
1512+
Scheduler.unstable_advanceTime(1000);
1513+
return <ClassComponent ref={refSetter} />;
1514+
};
1515+
1516+
Scheduler.unstable_advanceTime(1);
1517+
1518+
const renderer = ReactTestRenderer.create(
1519+
<React.Profiler id="root" onCommit={callback}>
1520+
<Component />
1521+
</React.Profiler>,
1522+
);
1523+
1524+
expect(callback).toHaveBeenCalledTimes(1);
1525+
1526+
let call = callback.mock.calls[0];
1527+
1528+
expect(call).toHaveLength(enableSchedulerTracing ? 5 : 4);
1529+
expect(call[0]).toBe('root');
1530+
expect(call[1]).toBe('mount');
1531+
expect(call[2]).toBe(10); // durations
1532+
expect(call[3]).toBe(1001); // commit start time (before mutations or effects)
1533+
1534+
callback.mockClear();
1535+
1536+
renderer.update(<React.Profiler id="root" onCommit={callback} />);
1537+
1538+
expect(callback).toHaveBeenCalledTimes(1);
1539+
1540+
call = callback.mock.calls[0];
1541+
1542+
expect(call).toHaveLength(enableSchedulerTracing ? 5 : 4);
1543+
expect(call[0]).toBe('root');
1544+
expect(call[1]).toBe('update');
1545+
expect(call[2]).toBe(100); // durations
1546+
expect(call[3]).toBe(1011); // commit start time (before mutations or effects)
1547+
});
1548+
14941549
it('should bubble time spent in layout effects to higher profilers', () => {
14951550
const callback = jest.fn();
14961551

0 commit comments

Comments
 (0)