@samitouri / QOS-React / commits / b38ac13f94

DevTools: Add post-commit hook (#21183)

I recently added UI for the Profiler's commit and post-commit durations to the DevTools, but I made two pretty silly oversights: 1. I used the commit hook (called after mutation+layout effects) to read both the layout and passive effect durations. This is silly because passive effects may not have flushed yet git at this point. 2. I didn't reset the values on the HostRoot node, so they accumulated with each commit. This commitR addresses both issues: 1. First it adds a new DevTools hook, onPostCommitRoot*, to be called after passive effects get flushed. This gives DevTools the opportunity to read passive effect durations (if the build of React being profiled supports it). 2. Second the work loop resets these durations (on the HostRoot) after calling the post-commit hook so address the accumulation problem. I've also added a unit test to guard against this regressing in the future. * Doing this in flushPassiveEffectsImpl seemed simplest, since there are so many places we flush passive effects. Is there any potential problem with this though?

Brian Vaughn committed Apr 8, 2021 at 22:04 UTC b38ac13f946ad305ead05a6a83511549db27322e

11 files changed +275 -43

packages/react-devtools-shared/src/__tests__/profilingHostRoot-test.js new

+144

@@ -0,0 +1,144 @@

1 +/**

2 + * Copyright (c) Facebook, Inc. and its affiliates.

3 + *

4 + * This source code is licensed under the MIT license found in the

5 + * LICENSE file in the root directory of this source tree.

6 + *

7 + * @flow

8 + */

9 +

10 +describe('profiling HostRoot', () => {

11 + let React;

12 + let ReactDOM;

13 + let Scheduler;

14 + let store: Store;

15 + let utils;

16 + let getEffectDurations;

17 +

18 + let effectDurations;

19 + let passiveEffectDurations;

20 +

21 + beforeEach(() => {

22 + utils = require('./utils');

23 + utils.beforeEachProfiling();

24 +

25 + getEffectDurations = require('../backend/utils').getEffectDurations;

26 +

27 + store = global.store;

28 +

29 + React = require('react');

30 + ReactDOM = require('react-dom');

31 + Scheduler = require('scheduler');

32 +

33 + effectDurations = [];

34 + passiveEffectDurations = [];

35 +

36 + // This is the DevTools hook installed by the env.beforEach()

37 + // The hook is installed as a read-only property on the window,

38 + // so for our test purposes we can just override the commit hook.

39 + const hook = global.__REACT_DEVTOOLS_GLOBAL_HOOK__;

40 + hook.onPostCommitFiberRoot = function onPostCommitFiberRoot(

41 + rendererID,

42 + root,

43 + ) {

44 + const {effectDuration, passiveEffectDuration} = getEffectDurations(root);

45 + effectDurations.push(effectDuration);

46 + passiveEffectDurations.push(passiveEffectDuration);

47 + };

48 + });

49 +

50 + it('should expose passive and layout effect durations for render()', () => {

51 + function App() {

52 + React.useEffect(() => {

53 + Scheduler.unstable_advanceTime(10);

54 + });

55 + React.useLayoutEffect(() => {

56 + Scheduler.unstable_advanceTime(100);

57 + });

58 + return null;

59 + }

60 +

61 + utils.act(() => store.profilerStore.startProfiling());

62 + utils.act(() => {

63 + const container = document.createElement('div');

64 + ReactDOM.render(<App />, container);

65 + });

66 + utils.act(() => store.profilerStore.stopProfiling());

67 +

68 + expect(effectDurations).toHaveLength(1);

69 + const effectDuration = effectDurations[0];

70 + expect(effectDuration === null || effectDuration === 100).toBe(true);

71 + expect(passiveEffectDurations).toHaveLength(1);

72 + const passiveEffectDuration = passiveEffectDurations[0];

73 + expect(passiveEffectDuration === null || passiveEffectDuration === 10).toBe(

74 + true,

75 + );

76 + });

77 +

78 + it('should expose passive and layout effect durations for createRoot()', () => {

79 + function App() {

80 + React.useEffect(() => {

81 + Scheduler.unstable_advanceTime(10);

82 + });

83 + React.useLayoutEffect(() => {

84 + Scheduler.unstable_advanceTime(100);

85 + });

86 + return null;

87 + }

88 +

89 + utils.act(() => store.profilerStore.startProfiling());

90 + utils.act(() => {

91 + const container = document.createElement('div');

92 + const root = ReactDOM.unstable_createRoot(container);

93 + root.render(<App />);

94 + });

95 + utils.act(() => store.profilerStore.stopProfiling());

96 +

97 + expect(effectDurations).toHaveLength(1);

98 + const effectDuration = effectDurations[0];

99 + expect(effectDuration === null || effectDuration === 100).toBe(true);

100 + expect(passiveEffectDurations).toHaveLength(1);

101 + const passiveEffectDuration = passiveEffectDurations[0];

102 + expect(passiveEffectDuration === null || passiveEffectDuration === 10).toBe(

103 + true,

104 + );

105 + });

106 +

107 + it('should properly reset passive and layout effect durations between commits', () => {

108 + function App({shouldCascade}) {

109 + const [, setState] = React.useState(false);

110 + React.useEffect(() => {

111 + Scheduler.unstable_advanceTime(10);

112 + });

113 + React.useLayoutEffect(() => {

114 + Scheduler.unstable_advanceTime(100);

115 + });

116 + React.useLayoutEffect(() => {

117 + if (shouldCascade) {

118 + setState(true);

119 + }

120 + }, [shouldCascade]);

121 + return null;

122 + }

123 +

124 + const container = document.createElement('div');

125 + const root = ReactDOM.unstable_createRoot(container);

126 +

127 + utils.act(() => store.profilerStore.startProfiling());

128 + utils.act(() => root.render(<App />));

129 + utils.act(() => root.render(<App shouldCascade={true} />));

130 + utils.act(() => store.profilerStore.stopProfiling());

131 +

132 + expect(effectDurations).toHaveLength(3);

133 + expect(passiveEffectDurations).toHaveLength(3);

134 +

135 + for (let i = 0; i < effectDurations.length; i++) {

136 + const effectDuration = effectDurations[i];

137 + expect(effectDuration === null || effectDuration === 100).toBe(true);

138 + const passiveEffectDuration = passiveEffectDurations[i];

139 + expect(

140 + passiveEffectDuration === null || passiveEffectDuration === 10,

141 + ).toBe(true);

142 + }

143 + });

144 +});

packages/react-devtools-shared/src/backend/legacy/renderer.js

@@ -1012,6 +1012,9 @@ export function attach(

1012 const handleCommitFiberUnmount = () => {

1013 throw new Error('handleCommitFiberUnmount not supported by this renderer');

1014 };

1015 + const handlePostCommitFiberRoot = () => {

1016 + throw new Error('handlePostCommitFiberRoot not supported by this renderer');

1017 + };

1018 const overrideSuspense = () => {

1019 throw new Error('overrideSuspense not supported by this renderer');

1020 };

@@ -1082,6 +1085,7 @@ export function attach(

1085 getProfilingData,

1086 handleCommitFiberRoot,

1087 handleCommitFiberUnmount,

1088 + handlePostCommitFiberRoot,

1089 inspectElement,

1090 logElementToConsole,

1091 overrideSuspense,

packages/react-devtools-shared/src/backend/renderer.js

+25 -40

@@ -42,6 +42,7 @@ import {

42 copyWithDelete,

43 copyWithRename,

44 copyWithSet,

45 + getEffectDurations,

46 } from './utils';

47 import {

48 __DEBUG__,

@@ -369,6 +370,7 @@ export function getInternalReactConstants(

370 LegacyHiddenComponent,

371 MemoComponent,

372 OffscreenComponent,

373 + Profiler,

374 ScopeComponent,

375 SimpleMemoComponent,

376 SuspenseComponent,

@@ -442,6 +444,8 @@ export function getInternalReactConstants(

444 return 'Scope';

445 case SuspenseListComponent:

446 return 'SuspenseList';

447 + case Profiler:

448 + return 'Profiler';

449 default:

450 const typeSymbol = getTypeSymbol(type);

451

@@ -2154,25 +2158,6 @@ export function attach(

2158 // Checking root.memoizedInteractions handles multi-renderer edge-case-

2159 // where some v16 renderers support profiling and others don't.

2160 if (isProfiling && root.memoizedInteractions != null) {

2157 - // Profiling durations are only available for certain builds.

2158 - // If available, they'll be stored on the HostRoot.

2159 - let effectDuration = null;

2160 - let passiveEffectDuration = null;

2161 - const hostRoot = root.current;

2162 - if (hostRoot != null) {

2163 - const stateNode = hostRoot.stateNode;

2164 - if (stateNode != null) {

2165 - effectDuration =

2166 - stateNode.effectDuration != null

2167 - ? stateNode.effectDuration

2168 - : null;

2169 - passiveEffectDuration =

2170 - stateNode.passiveEffectDuration != null

2171 - ? stateNode.passiveEffectDuration

2172 - : null;

2173 - }

2174 - }

2175 -

2161 // If profiling is active, store commit time and duration, and the current interactions.

2162 // The frontend may request this information after profiling has stopped.

2163 currentCommitProfilingMetadata = {

@@ -2187,8 +2172,8 @@ export function attach(

2172 ),

2173 maxActualDuration: 0,

2174 priorityLevel: null,

2190 - effectDuration,

2191 - passiveEffectDuration,

2175 + effectDuration: null,

2176 + passiveEffectDuration: null,

2177 };

2178 }

2179

@@ -2206,6 +2191,19 @@ export function attach(

2191 recordUnmount(fiber, false);

2192 }

2193

2194 + function handlePostCommitFiberRoot(root) {

2195 + const isProfilingSupported = root.memoizedInteractions != null;

2196 + if (isProfiling && isProfilingSupported) {

2197 + if (currentCommitProfilingMetadata !== null) {

2198 + const {effectDuration, passiveEffectDuration} = getEffectDurations(

2199 + root,

2200 + );

2201 + currentCommitProfilingMetadata.effectDuration = effectDuration;

2202 + currentCommitProfilingMetadata.passiveEffectDuration = passiveEffectDuration;

2203 + }

2204 + }

2205 + }

2206 +

2207 function handleCommitFiberRoot(root, priorityLevel) {

2208 const current = root.current;

2209 const alternate = current.alternate;

@@ -2227,23 +2225,6 @@ export function attach(

2225 const isProfilingSupported = root.memoizedInteractions != null;

2226

2227 if (isProfiling && isProfilingSupported) {

2230 - // Profiling durations are only available for certain builds.

2231 - // If available, they'll be stored on the HostRoot.

2232 - let effectDuration = null;

2233 - let passiveEffectDuration = null;

2234 - const hostRoot = root.current;

2235 - if (hostRoot != null) {

2236 - const stateNode = hostRoot.stateNode;

2237 - if (stateNode != null) {

2238 - effectDuration =

2239 - stateNode.effectDuration != null ? stateNode.effectDuration : null;

2240 - passiveEffectDuration =

2241 - stateNode.passiveEffectDuration != null

2242 - ? stateNode.passiveEffectDuration

2243 - : null;

2244 - }

2245 - }

2246 -

2228 // If profiling is active, store commit time and duration, and the current interactions.

2229 // The frontend may request this information after profiling has stopped.

2230 currentCommitProfilingMetadata = {

@@ -2259,8 +2240,11 @@ export function attach(

2240 maxActualDuration: 0,

2241 priorityLevel:

2242 priorityLevel == null ? null : formatPriorityLevel(priorityLevel),

2262 - effectDuration,

2263 - passiveEffectDuration,

2243 +

2244 + // Initialize to null; if new enough React version is running,

2245 + // these values will be read during separate handlePostCommitFiberRoot() call.

2246 + effectDuration: null,

2247 + passiveEffectDuration: null,

2248 };

2249 }

2250

@@ -3856,6 +3840,7 @@ export function attach(

3840 getProfilingData,

3841 handleCommitFiberRoot,

3842 handleCommitFiberUnmount,

3843 + handlePostCommitFiberRoot,

3844 inspectElement,

3845 logElementToConsole,

3846 prepareViewAttributeSource,

packages/react-devtools-shared/src/backend/types.js

@@ -326,6 +326,7 @@ export type RendererInterface = {

326 getPathForElement: (id: number) => Array<PathFrame> | null,

327 handleCommitFiberRoot: (fiber: Object, commitPriority?: number) => void,

328 handleCommitFiberUnmount: (fiber: Object) => void,

329 + handlePostCommitFiberRoot: (fiber: Object) => void,

330 inspectElement: (

331 requestID: number,

332 id: number,

packages/react-devtools-shared/src/backend/utils.js

+20

@@ -118,6 +118,26 @@ export function copyWithSet(

118 return updated;

119 }

120

121 +export function getEffectDurations(root: Object) {

122 + // Profiling durations are only available for certain builds.

123 + // If available, they'll be stored on the HostRoot.

124 + let effectDuration = null;

125 + let passiveEffectDuration = null;

126 + const hostRoot = root.current;

127 + if (hostRoot != null) {

128 + const stateNode = hostRoot.stateNode;

129 + if (stateNode != null) {

130 + effectDuration =

131 + stateNode.effectDuration != null ? stateNode.effectDuration : null;

132 + passiveEffectDuration =

133 + stateNode.passiveEffectDuration != null

134 + ? stateNode.passiveEffectDuration

135 + : null;

136 + }

137 + }

138 + return {effectDuration, passiveEffectDuration};

139 +}

140 +

141 export function serializeToString(data: any): string {

142 const cache = new Set();

143 // Use a custom replacer function to protect against circular references.

packages/react-devtools-shared/src/hook.js

@@ -287,6 +287,13 @@ export function installHook(target: any): DevToolsHook | null {

287 }

288 }

289

290 + function onPostCommitFiberRoot(rendererID, root) {

291 + const rendererInterface = rendererInterfaces.get(rendererID);

292 + if (rendererInterface != null) {

293 + rendererInterface.handlePostCommitFiberRoot(root);

294 + }

295 + }

296 +

297 // TODO: More meaningful names for "rendererInterfaces" and "renderers".

298 const fiberRoots = {};

299 const rendererInterfaces = new Map();

@@ -315,6 +322,7 @@ export function installHook(target: any): DevToolsHook | null {

322 checkDCE,

323 onCommitFiberUnmount,

324 onCommitFiberRoot,

325 + onPostCommitFiberRoot,

326 };

327

328 Object.defineProperty(

packages/react-devtools-shell/src/app/InteractionTracing/index.js

+13 -1

@@ -21,6 +21,14 @@ import {

21 unstable_wrap as wrap,

22 } from 'scheduler/tracing';

24 +function sleep(ms) {

25 + const start = performance.now();

26 + let now;

27 + do {

28 + now = performance.now();

29 + } while (now - ms < start);

30 +}

31 +

32 export default function InteractionTracing() {

33 const [count, setCount] = useState(0);

34 const [shouldCascade, setShouldCascade] = useState(false);

@@ -75,7 +83,11 @@ export default function InteractionTracing() {

83 }, [count, shouldCascade]);

85 useLayoutEffect(() => {

78 - Math.sqrt(100 * 100 * 100 * 100 * 100);

86 + sleep(150);

87 + });

88 +

89 + useEffect(() => {

90 + sleep(300);

91 });

93 return (

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

+18

@@ -134,6 +134,24 @@ export function onCommitRoot(root: FiberRoot, eventPriority: EventPriority) {

134 }

135 }

136

137 +export function onPostCommitRoot(root: FiberRoot) {

138 + if (

139 + injectedHook &&

140 + typeof injectedHook.onPostCommitFiberRoot === 'function'

141 + ) {

142 + try {

143 + injectedHook.onPostCommitFiberRoot(rendererID, root);

144 + } catch (err) {

145 + if (__DEV__) {

146 + if (!hasLoggedError) {

147 + hasLoggedError = true;

148 + console.error('React instrumentation encountered an error: %s', err);

149 + }

150 + }

151 + }

152 + }

153 +}

154 +

155 export function onCommitUnmount(fiber: Fiber) {

156 if (injectedHook && typeof injectedHook.onCommitFiberUnmount === 'function') {

157 try {

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

+18

@@ -134,6 +134,24 @@ export function onCommitRoot(root: FiberRoot, eventPriority: EventPriority) {

134 }

135 }

136

137 +export function onPostCommitRoot(root: FiberRoot) {

138 + if (

139 + injectedHook &&

140 + typeof injectedHook.onPostCommitFiberRoot === 'function'

141 + ) {

142 + try {

143 + injectedHook.onPostCommitFiberRoot(rendererID, root);

144 + } catch (err) {

145 + if (__DEV__) {

146 + if (!hasLoggedError) {

147 + hasLoggedError = true;

148 + console.error('React instrumentation encountered an error: %s', err);

149 + }

150 + }

151 + }

152 + }

153 +}

154 +

155 export function onCommitUnmount(fiber: Fiber) {

156 if (injectedHook && typeof injectedHook.onCommitFiberUnmount === 'function') {

157 try {

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

+12 -1

@@ -229,7 +229,10 @@ import {

229 hasCaughtError,

230 clearCaughtError,

231 } from 'shared/ReactErrorUtils';

232 -import {onCommitRoot as onCommitRootDevTools} from './ReactFiberDevToolsHook.new';

232 +import {

233 + onCommitRoot as onCommitRootDevTools,

234 + onPostCommitRoot as onPostCommitRootDevTools,

235 +} from './ReactFiberDevToolsHook.new';

236 import {onCommitRoot as onCommitRootTestSelector} from './ReactTestSelectors';

237

238 // Used by `act`

@@ -2156,6 +2159,14 @@ function flushPassiveEffectsImpl() {

2159 nestedPassiveUpdateCount =

2160 rootWithPendingPassiveEffects === null ? 0 : nestedPassiveUpdateCount + 1;

2161

2162 + // TODO: Move to commitPassiveMountEffects

2163 + onPostCommitRootDevTools(root);

2164 + if (enableProfilerTimer && enableProfilerCommitHooks) {

2165 + const stateNode = root.current.stateNode;

2166 + stateNode.effectDuration = 0;

2167 + stateNode.passiveEffectDuration = 0;

2168 + }

2169 +

2170 return true;

2171 }

2172

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