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?

Author: Brian Vaughn

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

@@ -0,0 +1,144 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+describe('profiling HostRoot',()=>{
+letReact;
+letReactDOM;
+letScheduler;
+ let store: Store;
+letutils;
+letgetEffectDurations;
+
+leteffectDurations;
+letpassiveEffectDurations;
+
+beforeEach(()=>{
+utils=require('./utils');
+utils.beforeEachProfiling();
+
+getEffectDurations=require('../backend/utils').getEffectDurations;
+
+store=global.store;
+
+React=require('react');
+ReactDOM=require('react-dom');
+Scheduler=require('scheduler');
+
+effectDurations=[];
+passiveEffectDurations=[];
+
+// This is the DevTools hook installed by the env.beforEach()
+// The hook is installed as a read-only property on the window,
+// so for our test purposes we can just override the commit hook.
+consthook=global.__REACT_DEVTOOLS_GLOBAL_HOOK__;
+hook.onPostCommitFiberRoot=functiononPostCommitFiberRoot(
+rendererID,
+root,
+){
+const{effectDuration, passiveEffectDuration}=getEffectDurations(root);
+effectDurations.push(effectDuration);
+passiveEffectDurations.push(passiveEffectDuration);
+};
+});
+
+it('should expose passive and layout effect durations for render()',()=>{
+functionApp(){
+React.useEffect(()=>{
+Scheduler.unstable_advanceTime(10);
+});
+React.useLayoutEffect(()=>{
+Scheduler.unstable_advanceTime(100);
+});
+returnnull;
+}
+
+utils.act(()=>store.profilerStore.startProfiling());
+utils.act(()=>{
+constcontainer=document.createElement('div');
+ReactDOM.render(<App/>,container);
+});
+utils.act(()=>store.profilerStore.stopProfiling());
+
+expect(effectDurations).toHaveLength(1);
+consteffectDuration=effectDurations[0];
+expect(effectDuration===null||effectDuration===100).toBe(true);
+expect(passiveEffectDurations).toHaveLength(1);
+constpassiveEffectDuration=passiveEffectDurations[0];
+expect(passiveEffectDuration===null||passiveEffectDuration===10).toBe(
+true,
+);
+});
+
+it('should expose passive and layout effect durations for createRoot()',()=>{
+functionApp(){
+React.useEffect(()=>{
+Scheduler.unstable_advanceTime(10);
+});
+React.useLayoutEffect(()=>{
+Scheduler.unstable_advanceTime(100);
+});
+returnnull;
+}
+
+utils.act(()=>store.profilerStore.startProfiling());
+utils.act(()=>{
+constcontainer=document.createElement('div');
+constroot=ReactDOM.unstable_createRoot(container);
+root.render(<App/>);
+});
+utils.act(()=>store.profilerStore.stopProfiling());
+
+expect(effectDurations).toHaveLength(1);
+consteffectDuration=effectDurations[0];
+expect(effectDuration===null||effectDuration===100).toBe(true);
+expect(passiveEffectDurations).toHaveLength(1);
+constpassiveEffectDuration=passiveEffectDurations[0];
+expect(passiveEffectDuration===null||passiveEffectDuration===10).toBe(
+true,
+);
+});
+
+it('should properly reset passive and layout effect durations between commits',()=>{
+functionApp({shouldCascade}){
+const[,setState]=React.useState(false);
+React.useEffect(()=>{
+Scheduler.unstable_advanceTime(10);
+});
+React.useLayoutEffect(()=>{
+Scheduler.unstable_advanceTime(100);
+});
+React.useLayoutEffect(()=>{
+if(shouldCascade){
+setState(true);
+}
+},[shouldCascade]);
+returnnull;
+}
+
+constcontainer=document.createElement('div');
+constroot=ReactDOM.unstable_createRoot(container);
+
+utils.act(()=>store.profilerStore.startProfiling());
+utils.act(()=>root.render(<App/>));
+utils.act(()=>root.render(<AppshouldCascade={true}/>));
+utils.act(()=>store.profilerStore.stopProfiling());
+
+expect(effectDurations).toHaveLength(3);
+expect(passiveEffectDurations).toHaveLength(3);
+
+for(leti=0;i<effectDurations.length;i++){
+consteffectDuration=effectDurations[i];
+expect(effectDuration===null||effectDuration===100).toBe(true);
+constpassiveEffectDuration=passiveEffectDurations[i];
+expect(
+passiveEffectDuration===null||passiveEffectDuration===10,
+).toBe(true);
+}
+});
+});

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

@@ -1012,6 +1012,9 @@ export function attach(
 consthandleCommitFiberUnmount=()=>{
 thrownewError('handleCommitFiberUnmount not supported by this renderer');
 };
+consthandlePostCommitFiberRoot=()=>{
+thrownewError('handlePostCommitFiberRoot not supported by this renderer');
+};
 constoverrideSuspense=()=>{
 thrownewError('overrideSuspense not supported by this renderer');
 };
@@ -1082,6 +1085,7 @@ export function attach(
 getProfilingData,
 handleCommitFiberRoot,
 handleCommitFiberUnmount,
+handlePostCommitFiberRoot,
 inspectElement,
 logElementToConsole,
 overrideSuspense,

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

@@ -42,6 +42,7 @@ import {
 copyWithDelete,
 copyWithRename,
 copyWithSet,
+getEffectDurations,
 }from'./utils';
 import{
 __DEBUG__,
@@ -369,6 +370,7 @@ export function getInternalReactConstants(
  LegacyHiddenComponent,
  MemoComponent,
  OffscreenComponent,
+ Profiler,
  ScopeComponent,
  SimpleMemoComponent,
  SuspenseComponent,
@@ -442,6 +444,8 @@ export function getInternalReactConstants(
 return'Scope';
 caseSuspenseListComponent:
 return'SuspenseList';
+caseProfiler:
+return'Profiler';
 default:
 consttypeSymbol=getTypeSymbol(type);
 
@@ -2154,25 +2158,6 @@ export function attach(
 // Checking root.memoizedInteractions handles multi-renderer edge-case-
 // where some v16 renderers support profiling and others don't.
 if(isProfiling&&root.memoizedInteractions!=null){
-// Profiling durations are only available for certain builds.
-// If available, they'll be stored on the HostRoot.
-leteffectDuration=null;
-letpassiveEffectDuration=null;
-consthostRoot=root.current;
-if(hostRoot!=null){
-conststateNode=hostRoot.stateNode;
-if(stateNode!=null){
-effectDuration=
-stateNode.effectDuration!=null
- ? stateNode.effectDuration
- : null;
-passiveEffectDuration=
-stateNode.passiveEffectDuration!=null
- ? stateNode.passiveEffectDuration
- : null;
-}
-}
-
 // If profiling is active, store commit time and duration, and the current interactions.
 // The frontend may request this information after profiling has stopped.
 currentCommitProfilingMetadata={
@@ -2187,8 +2172,8 @@ export function attach(
 ),
 maxActualDuration: 0,
 priorityLevel: null,
- effectDuration,
- passiveEffectDuration,
+effectDuration: null,
+passiveEffectDuration: null,
 };
 }
 
@@ -2206,6 +2191,19 @@ export function attach(
 recordUnmount(fiber,false);
 }
 
+functionhandlePostCommitFiberRoot(root){
+constisProfilingSupported=root.memoizedInteractions!=null;
+if(isProfiling&&isProfilingSupported){
+if(currentCommitProfilingMetadata!==null){
+const{effectDuration,passiveEffectDuration}=getEffectDurations(
+root,
+);
+currentCommitProfilingMetadata.effectDuration=effectDuration;
+currentCommitProfilingMetadata.passiveEffectDuration=passiveEffectDuration;
+}
+}
+}
+
 functionhandleCommitFiberRoot(root,priorityLevel){
 constcurrent=root.current;
 constalternate=current.alternate;
@@ -2227,23 +2225,6 @@ export function attach(
 constisProfilingSupported=root.memoizedInteractions!=null;
 
 if(isProfiling&&isProfilingSupported){
-// Profiling durations are only available for certain builds.
-// If available, they'll be stored on the HostRoot.
-leteffectDuration=null;
-letpassiveEffectDuration=null;
-consthostRoot=root.current;
-if(hostRoot!=null){
-conststateNode=hostRoot.stateNode;
-if(stateNode!=null){
-effectDuration=
-stateNode.effectDuration!=null ? stateNode.effectDuration : null;
-passiveEffectDuration=
-stateNode.passiveEffectDuration!=null
- ? stateNode.passiveEffectDuration
- : null;
-}
-}
-
 // If profiling is active, store commit time and duration, and the current interactions.
 // The frontend may request this information after profiling has stopped.
 currentCommitProfilingMetadata={
@@ -2259,8 +2240,11 @@ export function attach(
 maxActualDuration: 0,
 priorityLevel:
 priorityLevel==null ? null : formatPriorityLevel(priorityLevel),
- effectDuration,
- passiveEffectDuration,
+
+// Initialize to null; if new enough React version is running,
+// these values will be read during separate handlePostCommitFiberRoot() call.
+effectDuration: null,
+passiveEffectDuration: null,
 };
 }
 
@@ -3856,6 +3840,7 @@ export function attach(
 getProfilingData,
 handleCommitFiberRoot,
 handleCommitFiberUnmount,
+handlePostCommitFiberRoot,
 inspectElement,
 logElementToConsole,
 prepareViewAttributeSource,

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

@@ -326,6 +326,7 @@ export type RendererInterface = {
 getPathForElement: (id: number)=>Array<PathFrame>|null,
 handleCommitFiberRoot: (fiber: Object,commitPriority?: number)=>void,
 handleCommitFiberUnmount: (fiber: Object)=>void,
+handlePostCommitFiberRoot: (fiber: Object)=>void,
 inspectElement: (
 requestID: number,
 id: number,

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

@@ -118,6 +118,26 @@ export function copyWithSet(
 returnupdated;
 }
 
+exportfunctiongetEffectDurations(root: Object){
+// Profiling durations are only available for certain builds.
+// If available, they'll be stored on the HostRoot.
+leteffectDuration=null;
+letpassiveEffectDuration=null;
+consthostRoot=root.current;
+if(hostRoot!=null){
+conststateNode=hostRoot.stateNode;
+if(stateNode!=null){
+effectDuration=
+stateNode.effectDuration!=null ? stateNode.effectDuration : null;
+passiveEffectDuration=
+stateNode.passiveEffectDuration!=null
+ ? stateNode.passiveEffectDuration
+ : null;
+}
+}
+return{effectDuration, passiveEffectDuration};
+}
+
 exportfunctionserializeToString(data: any): string{
 constcache=newSet();
 // 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 {
 }
 }
 
+functiononPostCommitFiberRoot(rendererID,root){
+constrendererInterface=rendererInterfaces.get(rendererID);
+if(rendererInterface!=null){
+rendererInterface.handlePostCommitFiberRoot(root);
+}
+}
+
 // TODO: More meaningful names for "rendererInterfaces" and "renderers".
 constfiberRoots={};
 constrendererInterfaces=newMap();
@@ -315,6 +322,7 @@ export function installHook(target: any): DevToolsHook | null {
  checkDCE,
  onCommitFiberUnmount,
  onCommitFiberRoot,
+ onPostCommitFiberRoot,
 };
 
 Object.defineProperty(

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

@@ -21,6 +21,14 @@ import {
 unstable_wrapaswrap,
 }from'scheduler/tracing';
 
+functionsleep(ms){
+conststart=performance.now();
+letnow;
+do{
+now=performance.now();
+}while(now-ms<start);
+}
+
 exportdefaultfunctionInteractionTracing(){
 const[count,setCount]=useState(0);
 const[shouldCascade,setShouldCascade]=useState(false);
@@ -75,7 +83,11 @@ export default function InteractionTracing() {
 },[count,shouldCascade]);
 
 useLayoutEffect(()=>{
-Math.sqrt(100*100*100*100*100);
+sleep(150);
+});
+
+useEffect(()=>{
+sleep(300);
 });
 
 return(