Refactored Profiler tree to better work with suspense

src/devtools/views/DevTools.js

@@ -9,6 +9,7 @@ import Settings from './Settings/Settings';
 import TabBar from './TabBar';
 import { SettingsContextController } from './Settings/SettingsContext';
 import { TreeContextController } from './Elements/TreeContext';
+import { ProfilerContextController } from './Profiler/ProfilerContext';
 import ReactLogo from './ReactLogo';
 
 import styles from './DevTools.css';
@@ -104,27 +105,31 @@ export default function DevTools({
       <StoreContext.Provider value={store}>
         <SettingsContextController browserTheme={browserTheme}>
           <TreeContextController viewElementSource={viewElementSource}>
-            <div className={styles.DevTools}>
-              {showTabBar && (
-                <div className={styles.TabBar}>
-                  <ReactLogo />
-                  <span className={styles.DevToolsVersion}>
-                    {process.env.DEVTOOLS_VERSION}
-                  </span>
-                  <div className={styles.Spacer} />
-                  <TabBar
-                    currentTab={tab}
-                    id="DevTools"
-                    selectTab={setTab}
-                    size="large"
-                    tabs={
-                      supportsProfiling ? tabsWithProfiler : tabsWithoutProfiler
-                    }
-                  />
-                </div>
-              )}
-              <div className={styles.TabContent}>{tabElement}</div>
-            </div>
+            <ProfilerContextController>
+              <div className={styles.DevTools}>
+                {showTabBar && (
+                  <div className={styles.TabBar}>
+                    <ReactLogo />
+                    <span className={styles.DevToolsVersion}>
+                      {process.env.DEVTOOLS_VERSION}
+                    </span>
+                    <div className={styles.Spacer} />
+                    <TabBar
+                      currentTab={tab}
+                      id="DevTools"
+                      selectTab={setTab}
+                      size="large"
+                      tabs={
+                        supportsProfiling
+                          ? tabsWithProfiler
+                          : tabsWithoutProfiler
+                      }
+                    />
+                  </div>
+                )}
+                <div className={styles.TabContent}>{tabElement}</div>
+              </div>
+            </ProfilerContextController>
           </TreeContextController>
         </SettingsContextController>
       </StoreContext.Provider>

src/devtools/views/Profiler/FilterModal.js

@@ -1,7 +1,8 @@
 // @flow
 
-import React, { useCallback, useEffect, useRef } from 'react';
-import { useLocalStorage, useModalDismissSignal } from '../hooks';
+import React, { useCallback, useContext, useEffect, useRef } from 'react';
+import { ProfilerContext } from './ProfilerContext';
+import { useModalDismissSignal } from '../hooks';
 
 import styles from './FilterModal.css';
 
@@ -10,14 +11,12 @@ type Props = {|
 |};
 
 export default function FilterModal({ dismissModal }: Props) {
-  const [
+  const {
     isCommitFilterEnabled,
+    minCommitDuration,
     setIsCommitFilterEnabled,
-  ] = useLocalStorage<boolean>('isCommitFilterEnabled', false);
-  const [minCommitDuration, setMinCommitDuration] = useLocalStorage<number>(
-    'minCommitDuration',
-    0
-  );
+    setMinCommitDuration,
+  } = useContext(ProfilerContext);
 
   const handleNumberChange = useCallback(
     ({ currentTarget }) => {

src/devtools/views/Profiler/Profiler.js

@@ -1,11 +1,7 @@
 // @flow
 
 import React, { Suspense, useCallback, useContext, useState } from 'react';
-import { ProfilerDataContextController } from './ProfilerDataContext';
-import {
-  ProfilerStatusContext,
-  ProfilerStatusContextController,
-} from './ProfilerStatusContext';
+import { ProfilerContext } from './ProfilerContext';
 import Button from '../Button';
 import ButtonIcon from '../ButtonIcon';
 import TabBar from '../TabBar';
@@ -15,46 +11,68 @@ import SnapshotSelector from './SnapshotSelector';
 
 import styles from './Profiler.css';
 
-export type Props = {||};
+export default function Profiler(_: {||}) {
+  const { hasProfilingData, isProfiling } = useContext(ProfilerContext);
 
-export default function ProfilerOuter(_: Props) {
-  return (
-    <ProfilerStatusContextController>
+  if (isProfiling || !hasProfilingData) {
+    return <NonSuspendingProfiler isProfiling={isProfiling} />;
+  } else {
+    return (
       <Suspense fallback={<ProfilerFallback />}>
-        <ProfilerDataContextController>
-          <ProfilerInner />
-        </ProfilerDataContextController>
+        <SuspendingProfiler />
       </Suspense>
-    </ProfilerStatusContextController>
+    );
+  }
+}
+
+// This view is rendered when there is no profiler data (either we haven't profiled yet or we're currently profiling).
+// Nothing in this view's subtree suspends.
+// By not suspending while profiling is in progress, we avoid potential cache invalidation trickiness.
+function NonSuspendingProfiler({ isProfiling }: {| isProfiling: boolean |}) {
+  const view = isProfiling ? <RecortdingInProgress /> : <NoProfilingData />;
+
+  return (
+    <div className={styles.Profiler}>
+      <div className={styles.LeftColumn}>
+        <div className={styles.Toolbar}>
+          <RecordToggle />
+          <Button disabled title="Reload and start profiling">
+            {/* TODO (profiling) Wire up reload button */}
+            <ButtonIcon type="reload" />
+          </Button>
+          <div className={styles.VRule} />
+          <TabBar
+            currentTab={null}
+            disabled
+            id="Profiler"
+            selectTab={() => {}}
+            size="small"
+            tabs={tabs}
+          />
+        </div>
+        <div className={styles.Content}>{view}</div>
+      </div>
+    </div>
   );
 }
 
+// TODO (profiling) Real fallback UI
 function ProfilerFallback() {
-  // TODO (profiling) Real fallback UI
-  return null;
+  return <div>Loading...</div>;
 }
 
-function ProfilerInner(_: Props) {
-  const { hasProfilingData, isProfiling } = useContext(ProfilerStatusContext);
-
-  const showProfilingControls = !isProfiling && hasProfilingData;
-
+// This view is rendered when there is profiler data (even though there may not be any for the currently selected root).
+// This view's subtree uses suspense to request profiler data from the backend.
+function SuspendingProfiler(_: {||}) {
   const [tab, setTab] = useState('flame-chart');
   const [isFilterModalShowing, setIsFilterModalShowing] = useState(false);
 
   const showFilterModal = useCallback(() => setIsFilterModalShowing(true));
   const dismissFilterModal = useCallback(() => setIsFilterModalShowing(false));
 
-  let view = null;
-  if (isProfiling) {
-    view = <RecortdingInProgress />;
-  } else if (!hasProfilingData) {
-    view = <NoProfilingData />;
-  } else {
-    // TODO (profiling) Differentiate between no data and no data for the current root
-    // TODO (profiling) Show selected "tab" view
-    view = <div>Coming soon...</div>;
-  }
+  // TODO (profiling) Differentiate between no data and no data for the current root
+  // TODO (profiling) Show selected "tab" view
+  const view = <div>Coming soon...</div>;
 
   return (
     <div className={styles.Profiler}>
@@ -68,7 +86,6 @@ function ProfilerInner(_: Props) {
           <div className={styles.VRule} />
           <TabBar
             currentTab={tab}
-            disabled={!showProfilingControls}
             id="Profiler"
             selectTab={setTab}
             size="small"
@@ -78,7 +95,7 @@ function ProfilerInner(_: Props) {
           <Button onClick={showFilterModal} title="Filter commits by duration">
             <ButtonIcon type="filter" />
           </Button>
-          {showProfilingControls && <SnapshotSelector />}
+          <SnapshotSelector />
         </div>
         <div className={styles.Content}>
           {view}
@@ -87,13 +104,11 @@ function ProfilerInner(_: Props) {
           )}
         </div>
       </div>
-      {showProfilingControls && (
-        <div className={styles.RightColumn}>
-          {/* TODO (profiler) Dynamic information */}
-          <div className={styles.Toolbar}>Commit information</div>
-          <div className={styles.InspectedProperties} />
-        </div>
-      )}
+      <div className={styles.RightColumn}>
+        {/* TODO (profiler) Dynamic information */}
+        <div className={styles.Toolbar}>Commit information</div>
+        <div className={styles.InspectedProperties} />
+      </div>
     </div>
   );
 }

src/devtools/views/Profiler/ProfilerContext.js

@@ -0,0 +1,159 @@
+// @flow
+
+import React, {
+  createContext,
+  useCallback,
+  useContext,
+  useMemo,
+  useState,
+} from 'react';
+import { useLocalStorage, useSubscription } from '../hooks';
+import { TreeContext } from '../Elements/TreeContext';
+import { StoreContext } from '../context';
+import Store from '../../store';
+
+type Context = {|
+  // Have we recorded any profiling data?
+  // Are we currently profiling?
+  // This value may be modified by the record button in the Profiler toolbar,
+  // or from the backend itself (after a reload-and-profile action).
+  // It is synced between the backend and frontend via a Store subscription.
+  hasProfilingData: boolean,
+  isProfiling: boolean,
+  startProfiling(value: boolean): void,
+  stopProfiling(value: boolean): void,
+
+  // Which renderer and root should profiling data be shown for?
+  // Often this will correspond to the selected renderer and root in the Elements panel.
+  // If nothing is selected though, this will default to the first root.
+  rendererID: number | null,
+  rootID: number | null,
+
+  // Controls whether commits are filtered by duration.
+  // This value is controlled by a filter toggle UI in the Profiler toolbar.
+  // It impacts the commit selector UI as well as the fiber commits bar chart.
+  isCommitFilterEnabled: boolean,
+  setIsCommitFilterEnabled: (value: boolean) => void,
+  minCommitDuration: number,
+  setMinCommitDuration: (value: number) => void,
+
+  // Which commit is currently selected in the commit selector UI.
+  // Note that this is the index of the commit in all commits (non-filtered) that were profiled.
+  // This value is controlled by the commit selector UI in the Profiler toolbar.
+  // It impacts the flame graph and ranked charts.
+  selectedCommitIndex: number | null,
+  setSelectedCommitIndex: (value: number | null) => void,
+|};
+
+const ProfilerContext = createContext<Context>(((null: any): Context));
+ProfilerContext.displayName = 'ProfilerContext';
+
+type StoreProfilingState = {|
+  hasProfilingData: boolean,
+  isProfiling: boolean,
+|};
+
+type Props = {|
+  children: React$Node,
+|};
+
+function ProfilerContextController({ children }: Props) {
+  const store = useContext(StoreContext);
+  const { selectedElementID } = useContext(TreeContext);
+
+  const subscription = useMemo(
+    () => ({
+      getCurrentValue: () => ({
+        hasProfilingData: store.hasProfilingData,
+        isProfiling: store.isProfiling,
+      }),
+      subscribe: (callback: Function) => {
+        store.addListener('isProfiling', callback);
+        return () => store.removeListener('isProfiling', callback);
+      },
+    }),
+    [store]
+  );
+  const { isProfiling, hasProfilingData } = useSubscription<
+    StoreProfilingState,
+    Store
+  >(subscription);
+
+  // TODO (profiling) The browser extension is a multi-root app,
+  // so it won't work for the "Profiling" root to depend on a value that's set by the "Elements" root.
+  // We'll either need to lift that state up into the (shared) Store,
+  // or use a portal to share the contexts themselves between Chrome tabs.
+  let rendererID = null;
+  let rootID = null;
+  if (selectedElementID) {
+    rendererID = store.getRendererIDForElement(
+      ((selectedElementID: any): number)
+    );
+    rootID = store.getRootIDForElement(((selectedElementID: any): number));
+  } else if (store.roots.length > 0) {
+    // If no root is selected, assume the first root; many React apps are single root anyway.
+    rootID = store.roots[0];
+    rendererID = store.getRendererIDForElement(((rootID: any): number));
+  }
+
+  const startProfiling = useCallback(() => store.startProfiling(), [store]);
+  const stopProfiling = useCallback(() => store.stopProfiling(), [store]);
+
+  const [
+    isCommitFilterEnabled,
+    setIsCommitFilterEnabled,
+  ] = useLocalStorage<boolean>('isCommitFilterEnabled', false);
+  const [minCommitDuration, setMinCommitDuration] = useLocalStorage<number>(
+    'minCommitDuration',
+    0
+  );
+
+  const [selectedCommitIndex, setSelectedCommitIndex] = useState<number | null>(
+    null
+  );
+
+  const value = useMemo(
+    () => ({
+      hasProfilingData,
+      isProfiling,
+      startProfiling,
+      stopProfiling,
+
+      rendererID,
+      rootID,
+
+      isCommitFilterEnabled,
+      setIsCommitFilterEnabled,
+      minCommitDuration,
+      setMinCommitDuration,
+
+      selectedCommitIndex,
+      setSelectedCommitIndex,
+    }),
+    [
+      hasProfilingData,
+      isProfiling,
+      startProfiling,
+      stopProfiling,
+
+      rendererID,
+      rootID,
+
+      isCommitFilterEnabled,
+      setIsCommitFilterEnabled,
+      minCommitDuration,
+      setMinCommitDuration,
+
+      selectedCommitIndex,
+      setSelectedCommitIndex,
+    ]
+  );
+
+  return (
+    <ProfilerContext.Provider value={value}>
+      {children}
+    </ProfilerContext.Provider>
+  );
+}
+
+export { ProfilerContext, ProfilerContextController };

src/devtools/views/Profiler/ProfilerDataContext.js

@@ -1,94 +0,0 @@
-// @flow
-
-import React, { createContext, useContext, useMemo, useState } from 'react';
-import { TreeContext } from 'src/devtools/views/Elements/TreeContext';
-import { StoreContext } from '../context';
-import { useLocalStorage } from '../hooks';
-import { ProfilerStatusContext } from './ProfilerStatusContext';
-
-type Context = {|
-  commitIndex: number | null,
-  filteredCommitIndices: Array<number>,
-  rendererID: number | null,
-  rootID: number | null,
-  setCommitIndex: (value: number) => void,
-|};
-
-const ProfilerDataContext = createContext<Context>(((null: any): Context));
-ProfilerDataContext.displayName = 'ProfilerDataContext';
-
-type Props = {|
-  children: React$Node,
-|};
-
-function ProfilerDataContextController({ children }: Props) {
-  const store = useContext(StoreContext);
-
-  // TODO (profiling) The browser extension is a multi-root app,
-  // so it won't work for the "Profiling" root to depend on a value that's set by the "Elements" root.
-  // We'll either need to lift that state up into the (shared) Store,
-  // or use a portal to share the contexts themselves between Chrome tabs.
-  const { selectedElementID } = useContext(TreeContext);
-
-  // If no root is selected, assume the first root; many React apps are single root anyway.
-  let rendererID = null;
-  let rootID = null;
-  if (selectedElementID) {
-    rendererID = store.getRendererIDForElement(
-      ((selectedElementID: any): number)
-    );
-    rootID = store.getRootIDForElement(((selectedElementID: any): number));
-  } else if (store.roots.length > 0) {
-    rootID = store.roots[0];
-    rendererID = store.getRendererIDForElement(((rootID: any): number));
-  }
-
-  // This value is important because it ensure we re-render after our suspense cache has been cleared.
-  const { isProfiling } = useContext(ProfilerStatusContext);
-
-  const profilingSummary = store.profilingCache.ProfilingSummary.read({
-    rendererID: ((rendererID: any): number),
-    rootID: ((rootID: any): number),
-  });
-
-  const [isCommitFilterEnabled] = useLocalStorage<boolean>(
-    'isCommitFilterEnabled',
-    false
-  );
-  const [minCommitDuration] = useLocalStorage<number>('minCommitDuration', 0);
-  const { commitDurations } = profilingSummary;
-  const filteredCommitIndices = useMemo(() => {
-    const array = [];
-    if (!isProfiling) {
-      for (let i = 0; i < commitDurations.length; i++) {
-        if (!isCommitFilterEnabled || commitDurations[i] >= minCommitDuration) {
-          array.push(i);
-        }
-      }
-    }
-    return array;
-  }, [commitDurations, isCommitFilterEnabled, isProfiling, minCommitDuration]);
-
-  const [commitIndex, setCommitIndex] = useState<number | null>(
-    commitDurations.length > 0 ? 0 : null
-  );
-
-  const value = useMemo(
-    () => ({
-      commitIndex,
-      filteredCommitIndices,
-      rendererID,
-      rootID,
-      setCommitIndex,
-    }),
-    [commitIndex, filteredCommitIndices, rendererID, rootID, setCommitIndex]
-  );
-
-  return (
-    <ProfilerDataContext.Provider value={value}>
-      {children}
-    </ProfilerDataContext.Provider>
-  );
-}
-
-export { ProfilerDataContext, ProfilerDataContextController };

src/devtools/views/Profiler/ProfilerStatusContext.js

@@ -1,69 +0,0 @@
-// @flow
-
-import React, { createContext, useCallback, useContext, useMemo } from 'react';
-import { useSubscription } from '../hooks';
-import { StoreContext } from '../context';
-import Store from '../../store';
-
-type Context = {|
-  hasProfilingData: boolean,
-  isProfiling: boolean,
-  startProfiling(value: boolean): void,
-  stopProfiling(value: boolean): void,
-|};
-
-const ProfilerStatusContext = createContext<Context>(((null: any): Context));
-ProfilerStatusContext.displayName = 'ProfilerStatusContext';
-
-type StoreProfilingState = {|
-  hasProfilingData: boolean,
-  isProfiling: boolean,
-|};
-
-type Props = {|
-  children: React$Node,
-|};
-
-function ProfilerStatusContextController({ children }: Props) {
-  const store = useContext(StoreContext);
-
-  const subscription = useMemo(
-    () => ({
-      getCurrentValue: () => ({
-        hasProfilingData: store.hasProfilingData,
-        isProfiling: store.isProfiling,
-      }),
-      subscribe: (callback: Function) => {
-        store.addListener('isProfiling', callback);
-        return () => store.removeListener('isProfiling', callback);
-      },
-    }),
-    [store]
-  );
-
-  const { isProfiling, hasProfilingData } = useSubscription<
-    StoreProfilingState,
-    Store
-  >(subscription);
-
-  const startProfiling = useCallback(() => store.startProfiling(), [store]);
-  const stopProfiling = useCallback(() => store.stopProfiling(), [store]);
-
-  const value = useMemo(
-    () => ({
-      hasProfilingData,
-      isProfiling,
-      startProfiling,
-      stopProfiling,
-    }),
-    [hasProfilingData, isProfiling, startProfiling, stopProfiling]
-  );
-
-  return (
-    <ProfilerStatusContext.Provider value={value}>
-      {children}
-    </ProfilerStatusContext.Provider>
-  );
-}
-
-export { ProfilerStatusContext, ProfilerStatusContextController };

src/devtools/views/Profiler/RecordToggle.js

@@ -3,7 +3,7 @@
 import React, { useContext } from 'react';
 import Button from '../Button';
 import ButtonIcon from '../ButtonIcon';
-import { ProfilerStatusContext } from './ProfilerStatusContext';
+import { ProfilerContext } from './ProfilerContext';
 
 import styles from './RecordToggle.css';
 
@@ -11,7 +11,7 @@ export type Props = {||};
 
 export default function RecordToggle(_: Props) {
   const { isProfiling, startProfiling, stopProfiling } = useContext(
-    ProfilerStatusContext
+    ProfilerContext
   );
 
   return (

src/devtools/views/Profiler/SnapshotCommitList.js

@@ -2,7 +2,6 @@
 
 import React, {
   useCallback,
-  useContext,
   useEffect,
   useMemo,
   useRef,
@@ -12,72 +11,91 @@ import AutoSizer from 'react-virtualized-auto-sizer';
 import { FixedSizeList } from 'react-window';
 import SnapshotCommitListItem from './SnapshotCommitListItem';
 import { minBarWidth } from './constants';
-import { ProfilerDataContext } from './ProfilerDataContext';
-import { StoreContext } from '../context';
 
 export type ItemData = {|
   commitDurations: Array<number>,
-  commitIndex: number | null,
   commitTimes: Array<number>,
   filteredCommitIndices: Array<number>,
   isMouseDown: boolean,
   maxDuration: number,
-  setCommitIndex: (index: number) => void,
+  selectedCommitIndex: number | null,
+  setSelectedCommitIndex: (index: number) => void,
 |};
 
-type Props = {||};
+type Props = {|
+  commitDurations: Array<number>,
+  commitTimes: Array<number>,
+  filteredCommitIndices: Array<number>,
+  selectedCommitIndex: number | null,
+  setSelectedCommitIndex: (index: number) => void,
+|};
 
-export default function SnapshotCommitList(_: Props) {
+export default function SnapshotCommitList({
+  commitDurations,
+  commitTimes,
+  filteredCommitIndices,
+  selectedCommitIndex,
+  setSelectedCommitIndex,
+}: Props) {
   return (
     <AutoSizer>
-      {({ height, width }) => <List height={height} width={width} />}
+      {({ height, width }) => (
+        <List
+          commitDurations={commitDurations}
+          commitTimes={commitTimes}
+          height={height}
+          filteredCommitIndices={filteredCommitIndices}
+          selectedCommitIndex={selectedCommitIndex}
+          setSelectedCommitIndex={setSelectedCommitIndex}
+          width={width}
+        />
+      )}
     </AutoSizer>
   );
 }
 
 type ListProps = {|
+  commitDurations: Array<number>,
+  commitTimes: Array<number>,
   height: number,
+  filteredCommitIndices: Array<number>,
+  selectedCommitIndex: number | null,
+  setSelectedCommitIndex: (index: number) => void,
   width: number,
 |};
 
-function List({ height, width }: ListProps) {
+function List({
+  commitDurations,
+  selectedCommitIndex,
+  commitTimes,
+  height,
+  filteredCommitIndices,
+  setSelectedCommitIndex,
+  width,
+}: ListProps) {
   const listRef = useRef<FixedSizeList<ItemData> | null>(null);
-  const [isMouseDown, setIsMouseDown] = useState(false);
   const prevCommitIndexRef = useRef<number | null>(null);
 
-  const { profilingCache } = useContext(StoreContext);
-  const {
-    commitIndex,
-    filteredCommitIndices,
-    rendererID,
-    rootID,
-    setCommitIndex,
-  } = useContext(ProfilerDataContext);
-
-  const { commitDurations, commitTimes } = profilingCache.ProfilingSummary.read(
-    {
-      rendererID: ((rendererID: any): number),
-      rootID: ((rootID: any): number),
-    }
-  );
-
-  // Make sure any newly selected snapshot is visible within the list.
+  // Make sure a newly selected snapshot is fully visible within the list.
   useEffect(() => {
-    if (commitIndex !== prevCommitIndexRef.current) {
-      prevCommitIndexRef.current = commitIndex;
-      if (commitIndex !== null && listRef.current !== null) {
-        listRef.current.scrollToItem(commitIndex);
+    if (selectedCommitIndex !== prevCommitIndexRef.current) {
+      prevCommitIndexRef.current = selectedCommitIndex;
+      if (selectedCommitIndex !== null && listRef.current !== null) {
+        listRef.current.scrollToItem(selectedCommitIndex);
       }
     }
-  }, [listRef, commitIndex]);
+  }, [listRef, selectedCommitIndex]);
 
+  // When the mouse is down, dragging over a commit should auto-select it.
+  // This provides a nice way for users to swipe across a range of commits to compare them.
+  // TODO (profiling) This interaction may not feel as nice with suspense; reconsider it?
+  const [isMouseDown, setIsMouseDown] = useState(false);
   const handleMouseDown = useCallback(() => {
     setIsMouseDown(true);
   }, []);
   const handleMouseUp = useCallback(() => {
     setIsMouseDown(false);
   }, []);
-
   useEffect(() => {
     window.addEventListener('mouseup', handleMouseUp);
     return () => {
@@ -86,8 +104,8 @@ function List({ height, width }: ListProps) {
   }, [handleMouseUp]);
 
   const itemSize = useMemo(
-    () => Math.max(minBarWidth, width / commitDurations.length),
-    [commitDurations, width]
+    () => Math.max(minBarWidth, width / filteredCommitIndices.length),
+    [filteredCommitIndices, width]
   );
   const maxDuration = useMemo(
     () =>
@@ -102,21 +120,21 @@ function List({ height, width }: ListProps) {
   const itemData = useMemo<ItemData>(
     () => ({
       commitDurations,
-      commitIndex,
       commitTimes,
       filteredCommitIndices,
       isMouseDown,
       maxDuration,
-      setCommitIndex,
+      selectedCommitIndex,
+      setSelectedCommitIndex,
     }),
     [
       commitDurations,
-      commitIndex,
       commitTimes,
       filteredCommitIndices,
       isMouseDown,
       maxDuration,
-      setCommitIndex,
+      selectedCommitIndex,
+      setSelectedCommitIndex,
     ]
   );
 

src/devtools/views/Profiler/SnapshotCommitListItem.js

@@ -17,12 +17,12 @@ type Props = {
 function SnapshotCommitListItem({ data: itemData, index, style }: Props) {
   const {
     commitDurations,
-    commitIndex,
     commitTimes,
     filteredCommitIndices,
     isMouseDown,
     maxDuration,
-    setCommitIndex,
+    selectedCommitIndex,
+    setSelectedCommitIndex,
   } = itemData;
 
   index = filteredCommitIndices[index];
@@ -30,15 +30,15 @@ function SnapshotCommitListItem({ data: itemData, index, style }: Props) {
   const commitDuration = commitDurations[index];
   const commitTime = commitTimes[index];
 
-  const handleClick = useCallback(() => setCommitIndex(index), [
+  const handleClick = useCallback(() => setSelectedCommitIndex(index), [
     index,
-    setCommitIndex,
+    setSelectedCommitIndex,
   ]);
 
   // Guard against commits with duration 0
   const percentage =
     Math.min(1, Math.max(0, commitDuration / maxDuration)) || 0;
-  const isSelected = commitIndex === index;
+  const isSelected = selectedCommitIndex === index;
 
   // Leave a 1px gap between snapshots
   const width = parseFloat(style.width) - 1;

src/devtools/views/Profiler/SnapshotSelector.css

@@ -1,21 +1,13 @@
-.SnapshotSelector {
-  flex: 1 1 auto;
-  display: flex;
-  flex-direction: row;
-  align-items: center;
-  color: var(--color-text-color);
-  margin-left: 0.5rem;
-}
-
 .Button {
+  flex: 0 0 auto;
   margin-left: 0.25rem;
 }
 
 .Commits {
-  flex: 1 1 auto;
   height: 2.25rem;
-  min-width: 50px;
+  min-width: 30px;
   margin-left: 0.25rem;
+  overflow: hidden;
 }
 
 .VRule {
@@ -25,7 +17,8 @@
   margin: 0 0.25rem;
 }
 
-.Number {
+.IndexLabel {
+  flex: 0 0 auto;
   white-space: nowrap;
   font-family: var(--font-family-monospace);
 }

src/devtools/views/Profiler/SnapshotSelector.js

@@ -1,10 +1,12 @@
 // @flow
 
-import React, { Fragment, useCallback, useContext } from 'react';
+import React, { Fragment, useCallback, useContext, useMemo } from 'react';
 import Button from '../Button';
 import ButtonIcon from '../ButtonIcon';
-import { ProfilerDataContext } from './ProfilerDataContext';
+import { ProfilerContext } from './ProfilerContext';
 import SnapshotCommitList from './SnapshotCommitList';
+import { maxBarWidth } from './constants';
+import { StoreContext } from '../context';
 
 import styles from './SnapshotSelector.css';
 
@@ -12,17 +14,37 @@ export type Props = {||};
 
 export default function SnapshotSelector(_: Props) {
   const {
-    commitIndex,
-    filteredCommitIndices,
+    isCommitFilterEnabled,
+    minCommitDuration,
     rendererID,
     rootID,
-    setCommitIndex,
-  } = useContext(ProfilerDataContext);
+    selectedCommitIndex,
+    setSelectedCommitIndex,
+  } = useContext(ProfilerContext);
+
+  const { profilingCache } = useContext(StoreContext);
+  const { commitDurations, commitTimes } = profilingCache.ProfilingSummary.read(
+    {
+      rendererID: ((rendererID: any): number),
+      rootID: ((rootID: any): number),
+    }
+  );
+
+  const filteredCommitIndices = useMemo(
+    () =>
+      commitDurations.reduce((reduced, commitDuration, index) => {
+        if (!isCommitFilterEnabled || commitDuration >= minCommitDuration) {
+          reduced.push(index);
+        }
+        return reduced;
+      }, []),
+    [commitDurations, isCommitFilterEnabled, minCommitDuration]
+  );
 
   const numCommits = filteredCommitIndices.length;
   let currentCommitNumber = '-';
   if (numCommits > 0) {
-    currentCommitNumber = `${commitIndex + 1}`.padStart(
+    currentCommitNumber = `${selectedCommitIndex + 1}`.padStart(
       `${numCommits}`.length,
       '0'
     );
@@ -30,15 +52,18 @@ export default function SnapshotSelector(_: Props) {
 
   const viewNextCommit = useCallback(() => {
     const nextCommitIndex = Math.min(
-      ((commitIndex: any): number) + 1,
+      ((selectedCommitIndex: any): number) + 1,
       filteredCommitIndices.length - 1
     );
-    setCommitIndex(filteredCommitIndices[nextCommitIndex]);
-  }, [commitIndex, filteredCommitIndices, setCommitIndex]);
+    setSelectedCommitIndex(filteredCommitIndices[nextCommitIndex]);
+  }, [selectedCommitIndex, filteredCommitIndices, setSelectedCommitIndex]);
   const viewPrevCommit = useCallback(() => {
-    const nextCommitIndex = Math.max(((commitIndex: any): number) - 1, 0);
-    setCommitIndex(filteredCommitIndices[nextCommitIndex]);
-  }, [commitIndex, filteredCommitIndices, setCommitIndex]);
+    const nextCommitIndex = Math.max(
+      ((selectedCommitIndex: any): number) - 1,
+      0
+    );
+    setSelectedCommitIndex(filteredCommitIndices[nextCommitIndex]);
+  }, [selectedCommitIndex, filteredCommitIndices, setSelectedCommitIndex]);
 
   if (rendererID === null || rootID === null) {
     return null;
@@ -47,31 +72,43 @@ export default function SnapshotSelector(_: Props) {
   return (
     <Fragment>
       <div className={styles.VRule} />
-      <div className={styles.SnapshotSelector}>
-        <span className={styles.Number}>
-          {numCommits > 0 ? `${currentCommitNumber} / ${numCommits}` : '-'}
-        </span>
-        <Button
-          className={styles.Button}
-          disabled={commitIndex === null || commitIndex <= 0}
-          onClick={viewPrevCommit}
-        >
-          <ButtonIcon type="previous" />
-        </Button>
-        <div className={styles.Commits}>
-          {numCommits > 0 && <SnapshotCommitList />}
-          {numCommits === 0 && (
-            <div className={styles.NoCommits}>No commits</div>
-          )}
-        </div>
-        <Button
-          className={styles.Button}
-          disabled={commitIndex === null || commitIndex >= numCommits - 1}
-          onClick={viewNextCommit}
-        >
-          <ButtonIcon type="next" />
-        </Button>
+      <span className={styles.IndexLabel}>
+        {numCommits > 0 ? `${currentCommitNumber} / ${numCommits}` : '-'}
+      </span>
+      <Button
+        className={styles.Button}
+        disabled={selectedCommitIndex === 0 || numCommits === 0}
+        onClick={viewPrevCommit}
+      >
+        <ButtonIcon type="previous" />
+      </Button>
+      <div
+        className={styles.Commits}
+        style={{
+          flex: numCommits > 0 ? '1 1 auto' : '0 0 auto',
+          maxWidth: numCommits > 0 ? numCommits * maxBarWidth : undefined,
+        }}
+      >
+        {numCommits > 0 && (
+          <SnapshotCommitList
+            commitDurations={commitDurations}
+            commitTimes={commitTimes}
+            filteredCommitIndices={filteredCommitIndices}
+            selectedCommitIndex={selectedCommitIndex}
+            setSelectedCommitIndex={setSelectedCommitIndex}
+          />
+        )}
+        {numCommits === 0 && <div className={styles.NoCommits}>No commits</div>}
       </div>
+      <Button
+        className={styles.Button}
+        disabled={
+          selectedCommitIndex === null || selectedCommitIndex >= numCommits - 1
+        }
+        onClick={viewNextCommit}
+      >
+        <ButtonIcon type="next" />
+      </Button>
     </Fragment>
   );
 }

src/devtools/views/Profiler/constants.js

@@ -1,5 +1,6 @@
 // @flow
 
+export const maxBarWidth = 30;
 export const minBarHeight = 5;
 export const minBarWidth = 5;
 export const textHeight = 18;

src/devtools/views/hooks.js

@@ -2,15 +2,9 @@
 
 import { useCallback, useEffect, useLayoutEffect, useState } from 'react';
 
-type LocalStorageKey =
-  | 'displayDensity'
-  | 'isCommitFilterEnabled'
-  | 'minCommitDuration'
-  | 'theme';
-
 // Forked from https://usehooks.com/useLocalStorage/
 export function useLocalStorage<T>(
-  key: LocalStorageKey,
+  key: string,
   initialValue: T
 ): [T, (value: T | (() => T)) => void] {
   const getValueFromLocalStorage = useCallback(() => {