From aea780c619edd780fab967cecdfa55e2d83dd8ff Mon Sep 17 00:00:00 2001 From: Krishna Ravishankar <25255817+krsh732@users.noreply.github.com> Date: Mon, 27 Mar 2023 13:33:51 -0400 Subject: [PATCH] Add memoized selectors for hasUseful* --- src/components/shared/StackSettings.js | 41 +++++++++++--------------- src/profile-logic/profile-data.js | 6 ++-- src/selectors/per-thread/composed.js | 2 +- src/selectors/per-thread/thread.js | 25 ++++++++++++++++ src/test/store/useful-tabs.test.js | 6 ++-- 5 files changed, 50 insertions(+), 30 deletions(-) diff --git a/src/components/shared/StackSettings.js b/src/components/shared/StackSettings.js index 69ffb6d00f..8d29f1db48 100644 --- 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 index 2dc36aa4cb..f13ff60f5d 100644 --- 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 index 95f7f74e29..a848128fd6 100644 --- 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 index 9e038f803b..0d4da59e77 100644 --- 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 = ( state ) => getThread(state).nativeAllocations; + const getJsAllocations: Selector = (state) => + getThread(state).jsAllocations; const getThreadRange: Selector = (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 = createSelector( + getSamplesTable, + getThread, + ProfileData.hasUsefulSamples + ); + + const getHasUsefulJsAllocations: Selector = createSelector( + getJsAllocations, + getThread, + ProfileData.hasUsefulSamples + ); + + const getHasUsefulNativeAllocations: Selector = 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 index 5e7e6ed82b..8e4241179f 100644 --- 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',