Add memoized selectors for hasUseful*

firefox-devtools · Mar 27, 2023 · aea780c · aea780c
1 parent 7eac8d8
commit aea780c
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 30 deletions.
diff --git a/src/components/shared/StackSettings.js b/src/components/shared/StackSettings.js
@@ -24,7 +24,6 @@ import {
 import { PanelSearch } from './PanelSearch';
 
 import {
-  hasUsefulSamples,
   toValidImplementationFilter,
   toValidCallTreeSummaryStrategy,
 } from 'firefox-profiler/profile-logic/profile-data';
@@ -271,28 +270,24 @@ export const StackSettings = explicitConnect<
   StateProps,
   DispatchProps
 >({
-  mapStateToProps: (state) => {
-    const thread = selectedThreadSelectors.getThread(state);
-    const { samples, jsAllocations, nativeAllocations } = thread;
-    return {
-      invertCallstack: getInvertCallstack(state),
-      selectedTab: getSelectedTab(state),
-      showUserTimings: getShowUserTimings(state),
-      implementationFilter: getImplementationFilter(state),
-      currentSearchString: getCurrentSearchString(state),
-      hasUsefulTimingSamples: hasUsefulSamples(samples, thread),
-      hasUsefulJsAllocations:
-        jsAllocations !== undefined && hasUsefulSamples(jsAllocations, thread),
-      hasUsefulNativeAllocations:
-        nativeAllocations !== undefined &&
-        hasUsefulSamples(nativeAllocations, thread),
-      canShowRetainedMemory:
-        selectedThreadSelectors.getCanShowRetainedMemory(state),
-      callTreeSummaryStrategy:
-        selectedThreadSelectors.getCallTreeSummaryStrategy(state),
-      allowSwitchingStackType: getProfileUsesMultipleStackTypes(state),
-    };
-  },
+  mapStateToProps: (state) => ({
+    invertCallstack: getInvertCallstack(state),
+    selectedTab: getSelectedTab(state),
+    showUserTimings: getShowUserTimings(state),
+    implementationFilter: getImplementationFilter(state),
+    currentSearchString: getCurrentSearchString(state),
+    hasUsefulTimingSamples:
+      selectedThreadSelectors.getHasUsefulTimingSamples(state),
+    hasUsefulJsAllocations:
+      selectedThreadSelectors.getHasUsefulJsAllocations(state),
+    hasUsefulNativeAllocations:
+      selectedThreadSelectors.getHasUsefulNativeAllocations(state),
+    canShowRetainedMemory:
+      selectedThreadSelectors.getCanShowRetainedMemory(state),
+    callTreeSummaryStrategy:
+      selectedThreadSelectors.getCallTreeSummaryStrategy(state),
+    allowSwitchingStackType: getProfileUsesMultipleStackTypes(state),
+  }),
   mapDispatchToProps: {
     changeImplementationFilter,
     changeInvertCallstack,

diff --git a/src/profile-logic/profile-data.js b/src/profile-logic/profile-data.js
@@ -1280,11 +1280,11 @@ export function filterThreadByTab(
  * A useful sample being one that isn't a "(root)" sample.
  */
 export function hasUsefulSamples(
-  table: SamplesLikeTable,
+  table?: SamplesLikeTable,
   thread: Thread
 ): boolean {
   const { stackTable, frameTable, funcTable, stringTable } = thread;
-  if (table.length === 0 || stackTable.length === 0) {
+  if (table === undefined || table.length === 0 || stackTable.length === 0) {
     return false;
   }
   const stackIndex = table.stack.find((stack) => stack !== null);
@@ -1303,7 +1303,7 @@ export function hasUsefulSamples(
     if (stringTable.getString(stringIndex) === '(root)') {
       // If the first sample's stack is only the root, check if any other
       // sample is different.
-      return table.stack.some((s) => s !== stackIndex);
+      return table.stack.some((s) => s !== null && s !== stackIndex);
     }
   }
   return true;

diff --git a/src/selectors/per-thread/composed.js b/src/selectors/per-thread/composed.js
@@ -86,7 +86,7 @@ export function getComposedSelectorsPerThread(
 
       const { samples, jsAllocations, nativeAllocations } = thread;
       const hasSamples = [samples, jsAllocations, nativeAllocations].some(
-        (table) => table && hasUsefulSamples(table, thread)
+        (table) => hasUsefulSamples(table, thread)
       );
       if (!hasSamples) {
         visibleTabs = visibleTabs.filter(

diff --git a/src/selectors/per-thread/thread.js b/src/selectors/per-thread/thread.js
@@ -28,6 +28,7 @@ import type {
   JsTracerTable,
   SamplesTable,
   NativeAllocationsTable,
+  JsAllocationsTable,
   SamplesLikeTable,
   Selector,
   ThreadViewOptions,
@@ -88,6 +89,8 @@ export function getThreadSelectorsPerThread(
   const getNativeAllocations: Selector<NativeAllocationsTable | void> = (
     state
   ) => getThread(state).nativeAllocations;
+  const getJsAllocations: Selector<JsAllocationsTable | void> = (state) =>
+    getThread(state).jsAllocations;
   const getThreadRange: Selector<StartEndRange> = (state) =>
     // This function is already memoized in profile-data.js, so we don't need to
     // memoize it here with `createSelector`.
@@ -385,6 +388,24 @@ export function getThreadSelectorsPerThread(
     ProfileSelectors.getProfileViewOptions(state).perThread[threadsKey] ||
     defaultThreadViewOptions;
 
+  const getHasUsefulTimingSamples: Selector<boolean> = createSelector(
+    getSamplesTable,
+    getThread,
+    ProfileData.hasUsefulSamples
+  );
+
+  const getHasUsefulJsAllocations: Selector<boolean> = createSelector(
+    getJsAllocations,
+    getThread,
+    ProfileData.hasUsefulSamples
+  );
+
+  const getHasUsefulNativeAllocations: Selector<boolean> = createSelector(
+    getNativeAllocations,
+    getThread,
+    ProfileData.hasUsefulSamples
+  );
+
   /**
    * We can only compute the retained memory in the versions of the native allocations
    * format that provide the memory address. The earlier versions did not have
@@ -455,6 +476,7 @@ export function getThreadSelectorsPerThread(
     getSamplesTable,
     getSamplesWeightType,
     getNativeAllocations,
+    getJsAllocations,
     getThreadRange,
     getFilteredThread,
     getRangeFilteredThread,
@@ -474,6 +496,9 @@ export function getThreadSelectorsPerThread(
     getJsTracerTable,
     getExpensiveJsTracerTiming,
     getExpensiveJsTracerLeafTiming,
+    getHasUsefulTimingSamples,
+    getHasUsefulJsAllocations,
+    getHasUsefulNativeAllocations,
     getCanShowRetainedMemory,
     getCPUProcessedThread,
     getTabFilteredThread,

diff --git a/src/test/store/useful-tabs.test.js b/src/test/store/useful-tabs.test.js
@@ -103,9 +103,9 @@ describe('getUsefulTabs', function () {
 
   it('shows sample related tabs even when there are only allocation samples in the profile', function () {
     const { profile } = getProfileWithUnbalancedNativeAllocations();
-    profile.threads.forEach(
-      (thread) => (thread.samples = getEmptySamplesTableWithEventDelay())
-    );
+    for (const thread of profile.threads) {
+      thread.samples = getEmptySamplesTableWithEventDelay();
+    }
     const { getState } = storeWithProfile(profile);
     expect(selectedThreadSelectors.getUsefulTabs(getState())).toEqual([
       'calltree',