From 59456aa0dc69c84f65fca52261454ef50ce18182 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20Qu=C3=A8ze?= Date: Thu, 7 Jul 2022 23:48:17 +0200 Subject: [PATCH] Hide the user interface components showing stacks for tracks that don't have stack samples. --- src/actions/profile-view.js | 28 ++++++------- src/app-logic/tabs-handling.js | 6 +++ .../shared/SampleTooltipContents.js | 41 +++++++++++++++---- src/selectors/per-thread/composed.js | 39 +++++++++++++++++- .../SampleTooltipContents.test.js.snap | 18 ++++---- src/test/store/profile-view.test.js | 14 +++++-- src/test/store/useful-tabs.test.js | 28 +++++++++---- 7 files changed, 126 insertions(+), 48 deletions(-) diff --git a/src/actions/profile-view.js b/src/actions/profile-view.js index db78967c31..64e160a562 100644 --- a/src/actions/profile-view.js +++ b/src/actions/profile-view.js @@ -380,14 +380,13 @@ export function selectTrack( ); } - const doesNextTrackHaveSelectedTab = getThreadSelectors(selectedThreadIndex) - .getUsefulTabs(getState()) - .includes(selectedTab); - - if (!doesNextTrackHaveSelectedTab) { + const visibleTabs = getThreadSelectors(selectedThreadIndex).getUsefulTabs( + getState() + ); + if (!visibleTabs.includes(selectedTab)) { // If the user switches to another track that doesn't have the current - // selectedTab then switch to the calltree. - selectedTab = 'calltree'; + // selectedTab then switch to the first tab. + selectedTab = visibleTabs[0]; } let selectedThreadIndexes = new Set(getSelectedThreadIndexes(getState())); @@ -546,16 +545,13 @@ export function selectActiveTabTrack( ); } - const doesNextTrackHaveSelectedTab = getThreadSelectors( - selectedThreadIndexes - ) - .getUsefulTabs(getState()) - .includes(selectedTab); - - if (!doesNextTrackHaveSelectedTab) { + const visibleTabs = getThreadSelectors(selectedThreadIndexes).getUsefulTabs( + getState() + ); + if (!visibleTabs.includes(selectedTab)) { // If the user switches to another track that doesn't have the current - // selectedTab then switch to the calltree. - selectedTab = 'calltree'; + // selectedTab then switch to the first tab. + selectedTab = visibleTabs[0]; } if ( diff --git a/src/app-logic/tabs-handling.js b/src/app-logic/tabs-handling.js index 936dd44805..28850d0312 100644 --- a/src/app-logic/tabs-handling.js +++ b/src/app-logic/tabs-handling.js @@ -40,3 +40,9 @@ export const tabsWithTitleL10nIdArray: $ReadOnlyArray = name: tabSlug, title: tabsWithTitleL10nId[tabSlug], })); + +export const tabsShowingSampleData: $ReadOnlyArray = [ + 'calltree', + 'flame-graph', + 'stack-chart', +]; diff --git a/src/components/shared/SampleTooltipContents.js b/src/components/shared/SampleTooltipContents.js index 5ed9c76bc8..29d2f32fc6 100644 --- a/src/components/shared/SampleTooltipContents.js +++ b/src/components/shared/SampleTooltipContents.js @@ -7,7 +7,11 @@ import * as React from 'react'; import { Backtrace } from './Backtrace'; import { TooltipDetailSeparator } from '../tooltip/TooltipDetails'; -import { getCategoryPairLabel } from 'firefox-profiler/profile-logic/profile-data'; +import { + getCategoryPairLabel, + getFuncNamesAndOriginsForPath, + convertStackToCallNodeAndCategoryPath, +} from 'firefox-profiler/profile-logic/profile-data'; import { formatMilliseconds, formatPercent, @@ -20,6 +24,7 @@ import type { Thread, } from 'firefox-profiler/types'; import type { CpuRatioInTimeRange } from './thread/ActivityGraphFills'; +import { ensureExists } from '../../utils/flow'; type CPUProps = CpuRatioInTimeRange; @@ -52,7 +57,6 @@ class SampleTooltipCPUContents extends React.PureComponent {
CPU:
{cpuUsageAndPercentage}
-
); } @@ -117,6 +121,22 @@ export class SampleTooltipContents extends React.PureComponent { categories, implementationFilter, } = this.props; + + const { samples, stackTable } = rangeFilteredThread; + const stackIndex = samples.stack[sampleIndex]; + const hasSamples = samples.length > 0 && stackTable.length > 1; + let hasStack = false; + if (hasSamples) { + const stack = getFuncNamesAndOriginsForPath( + convertStackToCallNodeAndCategoryPath( + rangeFilteredThread, + ensureExists(stackIndex) + ), + rangeFilteredThread + ); + hasStack = stack.length > 1 || stack[0].funcName !== '(root)'; + } + return ( <> {cpuRatioInTimeRange === null ? null : ( @@ -125,12 +145,17 @@ export class SampleTooltipContents extends React.PureComponent { timeRange={cpuRatioInTimeRange.timeRange} /> )} - + {hasStack && cpuRatioInTimeRange !== null ? ( + + ) : null} + {!hasStack ? null : ( + + )} ); } diff --git a/src/selectors/per-thread/composed.js b/src/selectors/per-thread/composed.js index db3f565890..28ddc2b160 100644 --- a/src/selectors/per-thread/composed.js +++ b/src/selectors/per-thread/composed.js @@ -5,7 +5,11 @@ // @flow import { createSelector } from 'reselect'; -import { tabSlugs, type TabSlug } from '../../app-logic/tabs-handling'; +import { + tabSlugs, + type TabSlug, + tabsShowingSampleData, +} from '../../app-logic/tabs-handling'; import type { Selector, @@ -22,6 +26,8 @@ import type { StackTimingByDepth, } from '../../profile-logic/stack-timing'; +import { ensureExists } from '../../utils/flow'; + /** * Infer the return type from the getStackAndSampleSelectorsPerThread function. This * is done that so that the local type definition with `Selector` is the canonical @@ -59,7 +65,15 @@ export function getComposedSelectorsPerThread( threadSelectors.getThread, threadSelectors.getIsNetworkChartEmptyInFullRange, threadSelectors.getJsTracerTable, - ({ processType }, isNetworkChartEmpty, jsTracerTable) => { + (thread, isNetworkChartEmpty, jsTracerTable) => { + const { + processType, + samples, + stackTable, + stringTable, + frameTable, + funcTable, + } = thread; if (processType === 'comparison') { // For a diffing tracks, we display only the calltree tab for now, because // other views make no or not much sense. @@ -76,6 +90,27 @@ export function getComposedSelectorsPerThread( if (!jsTracerTable) { visibleTabs = visibleTabs.filter((tabSlug) => tabSlug !== 'js-tracer'); } + let hasSamples = samples.length > 0 && stackTable.length > 0; + if (hasSamples) { + const stackIndex = ensureExists(samples.stack[0]); + if (stackTable.prefix[stackIndex] === null) { + // There's only a single stack frame, check if it's '(root)'. + const frameIndex = stackTable.frame[stackIndex]; + const funcIndex = frameTable.func[frameIndex]; + const stringIndex = funcTable.name[funcIndex]; + if (stringTable.getString(stringIndex) === '(root)') { + // If the first sample's stack is only the root, check if any other + // sample is different. + hasSamples = samples.stack.some((s) => s !== stackIndex); + } + } + } + + if (!hasSamples) { + visibleTabs = visibleTabs.filter( + (tabSlug) => !tabsShowingSampleData.includes(tabSlug) + ); + } return visibleTabs; } ); diff --git a/src/test/components/__snapshots__/SampleTooltipContents.test.js.snap b/src/test/components/__snapshots__/SampleTooltipContents.test.js.snap index 80a24e7f0a..825501423e 100644 --- a/src/test/components/__snapshots__/SampleTooltipContents.test.js.snap +++ b/src/test/components/__snapshots__/SampleTooltipContents.test.js.snap @@ -298,10 +298,10 @@ exports[`SampleTooltipContents renders the sample with "variable CPU cycles" CPU
50% (average over 1.0ms)
-
+
@@ -410,10 +410,10 @@ exports[`SampleTooltipContents renders the sample with ns CPU usage information
70% (average over 1.0ms)
-
+
@@ -522,10 +522,10 @@ exports[`SampleTooltipContents renders the sample with µs CPU usage information
45% (average over 1.0ms)
-
+
diff --git a/src/test/store/profile-view.test.js b/src/test/store/profile-view.test.js index e7568f4f46..813482e1a4 100644 --- a/src/test/store/profile-view.test.js +++ b/src/test/store/profile-view.test.js @@ -441,12 +441,12 @@ describe('actions/ProfileView', function () { it('can switch back to the thread, which remembers the last viewed panel', function () { const profile = getNetworkTrackProfile(); const { dispatch, getState } = storeWithProfile(profile); - dispatch(App.changeSelectedTab('flame-graph')); + dispatch(App.changeSelectedTab('marker-table')); expect(UrlStateSelectors.getSelectedThreadIndexes(getState())).toEqual( new Set([0]) ); expect(UrlStateSelectors.getSelectedTab(getState())).toEqual( - 'flame-graph' + 'marker-table' ); dispatch(ProfileView.selectTrack(networkTrack, 'none')); expect(UrlStateSelectors.getSelectedThreadIndexes(getState())).toEqual( @@ -460,7 +460,7 @@ describe('actions/ProfileView', function () { new Set([0]) ); expect(UrlStateSelectors.getSelectedTab(getState())).toEqual( - 'flame-graph' + 'marker-table' ); }); }); @@ -519,9 +519,15 @@ describe('actions/ProfileView', function () { expect(UrlStateSelectors.getSelectedTab(getState())).toEqual( 'calltree' ); + // The thread with the memory track doesn't have samples, so switch to + // a tab that exists even for tables without samples. + dispatch(App.changeSelectedTab('marker-table')); + expect(UrlStateSelectors.getSelectedTab(getState())).toEqual( + 'marker-table' + ); dispatch(ProfileView.selectTrack(memoryTrackReference, 'none')); expect(UrlStateSelectors.getSelectedTab(getState())).toEqual( - 'calltree' + 'marker-table' ); }); }); diff --git a/src/test/store/useful-tabs.test.js b/src/test/store/useful-tabs.test.js index f93e91ca38..477283585e 100644 --- a/src/test/store/useful-tabs.test.js +++ b/src/test/store/useful-tabs.test.js @@ -7,6 +7,7 @@ import { selectedThreadSelectors } from '../../selectors/per-thread'; import { storeWithProfile } from '../fixtures/stores'; import { + getMarkerTableProfile, getProfileFromTextSamples, getProfileWithMarkers, getNetworkMarkers, @@ -33,9 +34,6 @@ describe('getUsefulTabs', function () { const profile = getProfileWithMarkers(getNetworkMarkers()); const { getState } = storeWithProfile(profile); expect(selectedThreadSelectors.getUsefulTabs(getState())).toEqual([ - 'calltree', - 'flame-graph', - 'stack-chart', 'marker-chart', 'marker-table', 'network-chart', @@ -46,9 +44,6 @@ describe('getUsefulTabs', function () { const profile = getProfileWithJsTracerEvents([['A', 0, 10]]); const { getState } = storeWithProfile(profile); expect(selectedThreadSelectors.getUsefulTabs(getState())).toEqual([ - 'calltree', - 'flame-graph', - 'stack-chart', 'marker-chart', 'marker-table', 'js-tracer', @@ -97,12 +92,27 @@ describe('getUsefulTabs', function () { // Check the tabs and make sure that the network chart is there. expect(selectedThreadSelectors.getUsefulTabs(getState())).toEqual([ - 'calltree', - 'flame-graph', - 'stack-chart', 'marker-chart', 'marker-table', 'network-chart', ]); }); + + it('hides sample related tabs when there is no sample data in the profile', function () { + const profile = getMarkerTableProfile(); + const { getState } = storeWithProfile(profile); + expect(selectedThreadSelectors.getUsefulTabs(getState())).toEqual([ + 'marker-chart', + 'marker-table', + ]); + }); + + it('hides sample related tabs when samples contain only the (root) frame', function () { + const { profile } = getProfileFromTextSamples('(root)'); + const { getState } = storeWithProfile(profile); + expect(selectedThreadSelectors.getUsefulTabs(getState())).toEqual([ + 'marker-chart', + 'marker-table', + ]); + }); });