diff --git a/src/profile-logic/merge-compare.js b/src/profile-logic/merge-compare.js index 2355652599..90c34b3bce 100644 --- a/src/profile-logic/merge-compare.js +++ b/src/profile-logic/merge-compare.js @@ -265,6 +265,12 @@ export function mergeProfilesForDiffing( ); } + // In merged profiles, we don't want to hide any threads: either they've been + // explicitely selected by the user, or it's the diffing track. + resultProfile.meta.initialVisibleThreads = resultProfile.threads.map( + (_, i) => i + ); + return { profile: resultProfile, implementationFilters, transformStacks }; } diff --git a/src/profile-logic/tracks.js b/src/profile-logic/tracks.js index 687dc1b9c1..cb6d41f262 100644 --- a/src/profile-logic/tracks.js +++ b/src/profile-logic/tracks.js @@ -993,7 +993,7 @@ function _computeThreadSampleScore( { samples, stackTable }: Thread, maxCpuDeltaPerInterval: number | null ): number { - if (samples.threadCPUDelta) { + if (meta.sampleUnits && samples.threadCPUDelta) { // Sum up all CPU deltas in this thread, to compute a total // CPU time for this thread (or a total CPU cycle count). return samples.threadCPUDelta.reduce( diff --git a/src/test/components/CallTreeSidebar.test.js b/src/test/components/CallTreeSidebar.test.js index ce0c34341d..9154eb29d2 100644 --- a/src/test/components/CallTreeSidebar.test.js +++ b/src/test/components/CallTreeSidebar.test.js @@ -158,7 +158,7 @@ describe('CallTreeSidebar', function () { getAllByText, funcNamesDict: { A, B, D }, } = setup( - getMergedProfileFromTextSamples( + getMergedProfileFromTextSamples([ ` A A A B B C @@ -168,8 +168,8 @@ describe('CallTreeSidebar', function () { A A A B B B G I E - ` - ) + `, + ]) ); dispatch(changeSelectedThreads(new Set([2]))); diff --git a/src/test/fixtures/profiles/processed-profile.js b/src/test/fixtures/profiles/processed-profile.js index a10cbddcac..8678958473 100644 --- a/src/test/fixtures/profiles/processed-profile.js +++ b/src/test/fixtures/profiles/processed-profile.js @@ -1042,7 +1042,13 @@ function _buildThreadFromTextOnlyStacks( /** * This returns a merged profile from a number of profile strings. */ -export function getMergedProfileFromTextSamples(...profileStrings: string[]): { +export function getMergedProfileFromTextSamples( + profileStrings: string[], + cpuValuesPerProfile: Array<{| + threadCPUDelta: Array, + threadCPUDeltaUnit: ThreadCPUDeltaUnit, + |} | null> = [] +): { profile: Profile, funcNamesPerThread: Array, funcNamesDictPerThread: Array<{ [funcName: string]: number }>, @@ -1051,6 +1057,16 @@ export function getMergedProfileFromTextSamples(...profileStrings: string[]): { getProfileFromTextSamples(str) ); const profiles = profilesAndFuncNames.map(({ profile }) => profile); + cpuValuesPerProfile.forEach((cpuValues, profileIndex) => { + if (cpuValues) { + addCpuUsageValues( + profiles[profileIndex], + cpuValues.threadCPUDelta, + cpuValues.threadCPUDeltaUnit + ); + } + }); + const profileState = stateFromLocation({ pathname: '/public/fakehash1/', search: '?thread=0&v=3', diff --git a/src/test/store/profile-view.test.js b/src/test/store/profile-view.test.js index 5d02a43a8f..a80f9020ac 100644 --- a/src/test/store/profile-view.test.js +++ b/src/test/store/profile-view.test.js @@ -540,10 +540,10 @@ describe('actions/ProfileView', function () { trackIndex: 2, }; - const { profile } = getMergedProfileFromTextSamples( + const { profile } = getMergedProfileFromTextSamples([ 'A B C', - 'A B B' - ); + 'A B B', + ]); const { getState, dispatch } = storeWithProfile(profile); dispatch(App.changeSelectedTab('flame-graph')); @@ -2900,18 +2900,18 @@ describe('getTimingsForSidebar', () => { describe('for a diffing track', function () { function setup() { const { profile, funcNamesDictPerThread } = - getMergedProfileFromTextSamples( + getMergedProfileFromTextSamples([ ` - A A A - B B C - D[cat:Layout] E F - `, + A A A + B B C + D[cat:Layout] E F + `, ` - A A A - B B B - Gjs I E - ` - ); + A A A + B B B + Gjs I E + `, + ]); const store = storeWithProfile(profile); store.dispatch(ProfileView.changeSelectedThreads(new Set([2]))); diff --git a/src/test/store/receive-profile.test.js b/src/test/store/receive-profile.test.js index 452023d21d..4b353a557c 100644 --- a/src/test/store/receive-profile.test.js +++ b/src/test/store/receive-profile.test.js @@ -3,9 +3,9 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ // @flow -import type { Profile } from 'firefox-profiler/types'; - import { oneLineTrim } from 'common-tags'; +import JSZip from 'jszip'; +import { indexedDB } from 'fake-indexeddb'; import { ensureExists } from 'firefox-profiler/utils/flow'; import { @@ -14,7 +14,6 @@ import { } from '../../profile-logic/data-structures'; import { getTimeRangeForThread } from '../../profile-logic/profile-data'; import { viewProfileFromPathInZipFile } from '../../actions/zipped-profiles'; -import { blankStore } from '../fixtures/stores'; import * as ProfileViewSelectors from '../../selectors/profile'; import * as ZippedProfilesSelectors from '../../selectors/zipped-profiles'; import * as UrlStateSelectors from '../../selectors/url-state'; @@ -35,10 +34,9 @@ import { changeTimelineTrackOrganization, } from '../../actions/receive-profile'; import { SymbolsNotFoundError } from '../../profile-logic/errors'; -import { indexedDB } from 'fake-indexeddb'; import { createGeckoProfile } from '../fixtures/profiles/gecko-profile'; -import JSZip from 'jszip'; +import { blankStore, storeWithProfile } from '../fixtures/stores'; import { makeProfileSerializable, processGeckoProfile, @@ -46,6 +44,7 @@ import { } from '../../profile-logic/process-profile'; import { getProfileFromTextSamples, + getMergedProfileFromTextSamples, addMarkersToThreadWithCorrespondingSamples, getProfileWithMarkers, getProfileWithThreadCPUDelta, @@ -58,6 +57,8 @@ import { waitUntilState } from '../fixtures/utils'; import { compress } from '../../utils/gz'; +import type { Profile } from 'firefox-profiler/types'; + // Mocking SymbolStoreDB. By default the functions will return undefined, which // will make the symbolication move forward with some bogus information. // If you need to simulate that it doesn't have the information, use the @@ -397,6 +398,22 @@ describe('actions/receive-profile', function () { ]); }); + it(`won't hide any tracks in a profile resulting from a compare operation`, () => { + const { profile } = getMergedProfileFromTextSamples([ + 'A', + 'A '.repeat(100), + ]); + + const store = storeWithProfile(profile); + + store.dispatch(viewProfile(profile)); + expect(getHumanReadableTracks(store.getState())).toEqual([ + 'show [thread Empty default] SELECTED', + 'show [thread Empty default]', + 'show [thread Diff between 1 and 2 comparison]', + ]); + }); + describe('with threadCPUDelta', function () { it('will show a thread when the relative CPU usage is above 10%', function () { const store = blankStore(); @@ -474,6 +491,31 @@ describe('actions/receive-profile', function () { ' - show [thread Thread with 10% CPU] SELECTED', // <- Ensure this thread is not hidden. ]); }); + + it(`won't hide any tracks in a profile resulting from a compare operation`, () => { + const { profile } = getMergedProfileFromTextSamples( + ['A A A A', 'B B B B'], + [ + { + threadCPUDelta: [10, 10, 10, 10_000_000], + threadCPUDeltaUnit: 'ns', + }, + { + threadCPUDelta: [10, 10_000_000, 10, 25], + threadCPUDeltaUnit: 'ns', + }, + ] + ); + + const store = storeWithProfile(profile); + + store.dispatch(viewProfile(profile)); + expect(getHumanReadableTracks(store.getState())).toEqual([ + 'show [thread Empty default] SELECTED', + 'show [thread Empty default]', + 'show [thread Diff between 1 and 2 comparison]', + ]); + }); }); describe('too many threads', function () { diff --git a/src/test/store/useful-tabs.test.js b/src/test/store/useful-tabs.test.js index 70fe17e467..96a2a890f8 100644 --- a/src/test/store/useful-tabs.test.js +++ b/src/test/store/useful-tabs.test.js @@ -51,7 +51,7 @@ describe('getUsefulTabs', function () { }); it('shows only the call tree when a diffing track is selected', function () { - const { profile } = getMergedProfileFromTextSamples('A B C', 'A B B'); + const { profile } = getMergedProfileFromTextSamples(['A B C', 'A B B']); const { getState, dispatch } = storeWithProfile(profile); expect(selectedThreadSelectors.getUsefulTabs(getState())).toEqual([ 'calltree', diff --git a/src/test/unit/profile-tree.test.js b/src/test/unit/profile-tree.test.js index 12a242ff21..284e4a4f6e 100644 --- a/src/test/unit/profile-tree.test.js +++ b/src/test/unit/profile-tree.test.js @@ -562,18 +562,18 @@ describe('inverted call tree', function () { describe('diffing trees', function () { function getProfile() { - const { profile } = getMergedProfileFromTextSamples( + const { profile } = getMergedProfileFromTextSamples([ ` - A A A - B B C - D E F - `, + A A A + B B C + D E F + `, ` - A A A - B B B - G I E - ` - ); + A A A + B B B + G I E + `, + ]); return profile; }