Skip to content

Commit 17a9140

Browse files
author
Brian Vaughn
committed
Profiler: Include ref callbacks in onCommit duration
1 parent c59c3df commit 17a9140

File tree

3 files changed

+169
-8
lines changed

3 files changed

+169
-8
lines changed

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

Lines changed: 57 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,14 +194,41 @@ 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+
try {
203+
startLayoutEffectTimer();
204+
invokeGuardedCallback(null, ref, null, null);
205+
} finally {
206+
recordLayoutEffectDuration(current);
207+
}
208+
} else {
209+
invokeGuardedCallback(null, ref, null, null);
210+
}
211+
198212
if (hasCaughtError()) {
199213
const refError = clearCaughtError();
200214
captureCommitPhaseError(current, nearestMountedAncestor, refError);
201215
}
202216
} else {
203217
try {
204-
ref(null);
218+
if (
219+
enableProfilerTimer &&
220+
enableProfilerCommitHooks &&
221+
current.mode & ProfileMode
222+
) {
223+
try {
224+
startLayoutEffectTimer();
225+
ref(null);
226+
} finally {
227+
recordLayoutEffectDuration(current);
228+
}
229+
} else {
230+
ref(null);
231+
}
205232
} catch (refError) {
206233
captureCommitPhaseError(current, nearestMountedAncestor, refError);
207234
}
@@ -965,7 +992,20 @@ function commitAttachRef(finishedWork: Fiber) {
965992
instanceToUse = instance;
966993
}
967994
if (typeof ref === 'function') {
968-
ref(instanceToUse);
995+
if (
996+
enableProfilerTimer &&
997+
enableProfilerCommitHooks &&
998+
finishedWork.mode & ProfileMode
999+
) {
1000+
try {
1001+
startLayoutEffectTimer();
1002+
ref(instanceToUse);
1003+
} finally {
1004+
recordLayoutEffectDuration(finishedWork);
1005+
}
1006+
} else {
1007+
ref(instanceToUse);
1008+
}
9691009
} else {
9701010
if (__DEV__) {
9711011
if (!ref.hasOwnProperty('current')) {
@@ -986,7 +1026,20 @@ function commitDetachRef(current: Fiber) {
9861026
const currentRef = current.ref;
9871027
if (currentRef !== null) {
9881028
if (typeof currentRef === 'function') {
989-
currentRef(null);
1029+
if (
1030+
enableProfilerTimer &&
1031+
enableProfilerCommitHooks &&
1032+
current.mode & ProfileMode
1033+
) {
1034+
try {
1035+
startLayoutEffectTimer();
1036+
currentRef(null);
1037+
} finally {
1038+
recordLayoutEffectDuration(current);
1039+
}
1040+
} else {
1041+
currentRef(null);
1042+
}
9901043
} else {
9911044
currentRef.current = null;
9921045
}

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

Lines changed: 57 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -180,14 +180,41 @@ 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+
try {
189+
startLayoutEffectTimer();
190+
invokeGuardedCallback(null, ref, null, null);
191+
} finally {
192+
recordLayoutEffectDuration(current);
193+
}
194+
} else {
195+
invokeGuardedCallback(null, ref, null, null);
196+
}
197+
184198
if (hasCaughtError()) {
185199
const refError = clearCaughtError();
186200
captureCommitPhaseError(current, refError);
187201
}
188202
} else {
189203
try {
190-
ref(null);
204+
if (
205+
enableProfilerTimer &&
206+
enableProfilerCommitHooks &&
207+
current.mode & ProfileMode
208+
) {
209+
try {
210+
startLayoutEffectTimer();
211+
ref(null);
212+
} finally {
213+
recordLayoutEffectDuration(current);
214+
}
215+
} else {
216+
ref(null);
217+
}
191218
} catch (refError) {
192219
captureCommitPhaseError(current, refError);
193220
}
@@ -832,7 +859,20 @@ function commitAttachRef(finishedWork: Fiber) {
832859
instanceToUse = instance;
833860
}
834861
if (typeof ref === 'function') {
835-
ref(instanceToUse);
862+
if (
863+
enableProfilerTimer &&
864+
enableProfilerCommitHooks &&
865+
finishedWork.mode & ProfileMode
866+
) {
867+
try {
868+
startLayoutEffectTimer();
869+
ref(instanceToUse);
870+
} finally {
871+
recordLayoutEffectDuration(finishedWork);
872+
}
873+
} else {
874+
ref(instanceToUse);
875+
}
836876
} else {
837877
if (__DEV__) {
838878
if (!ref.hasOwnProperty('current')) {
@@ -853,7 +893,20 @@ function commitDetachRef(current: Fiber) {
853893
const currentRef = current.ref;
854894
if (currentRef !== null) {
855895
if (typeof currentRef === 'function') {
856-
currentRef(null);
896+
if (
897+
enableProfilerTimer &&
898+
enableProfilerCommitHooks &&
899+
current.mode & ProfileMode
900+
) {
901+
try {
902+
startLayoutEffectTimer();
903+
currentRef(null);
904+
} finally {
905+
recordLayoutEffectDuration(current);
906+
}
907+
} else {
908+
currentRef(null);
909+
}
857910
} else {
858911
currentRef.current = null;
859912
}

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)