From c3f48b714fcf92ab34b4f05d0ac80f688ddef485 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Fri, 13 Jun 2025 12:04:38 +0200 Subject: [PATCH 1/8] Order the global threads by their main thread activity Previously we were ordering the global tracks their global track order by type. But if both global tracks were processes, we were simply keeping the initial thread order. Now we sort the process global tracks by their activity. --- src/actions/receive-profile.js | 12 ++++--- src/profile-logic/tracks.js | 61 +++++++++++++++++++++++++++++----- 2 files changed, 61 insertions(+), 12 deletions(-) diff --git a/src/actions/receive-profile.js b/src/actions/receive-profile.js index b7d26ce1d5..d397cab165 100644 --- a/src/actions/receive-profile.js +++ b/src/actions/receive-profile.js @@ -260,11 +260,13 @@ export function finalizeFullProfileView( ); const localTracksByPid = computeLocalTracksByPid(profile, globalTracks); + const threadActivityScores = getThreadActivityScores(getState()); const legacyThreadOrder = getLegacyThreadOrder(getState()); const globalTrackOrder = initializeGlobalTrackOrder( globalTracks, hasUrlInfo ? getGlobalTrackOrder(getState()) : null, - legacyThreadOrder + legacyThreadOrder, + threadActivityScores ); const localTrackOrderByPid = initializeLocalTrackOrderByPid( hasUrlInfo ? getLocalTrackOrderByPid(getState()) : null, @@ -308,7 +310,7 @@ export function finalizeFullProfileView( hiddenTracks = computeDefaultHiddenTracks( tracksWithOrder, profile, - getThreadActivityScores(getState()), + threadActivityScores, // Only include the parent process if there is no tab filter applied. includeParentProcessThreads ); @@ -1495,11 +1497,13 @@ export function changeTabFilter(tabID: TabID | null): ThunkAction { ); const localTracksByPid = computeLocalTracksByPid(profile, globalTracks); + const threadActivityScores = getThreadActivityScores(getState()); const legacyThreadOrder = getLegacyThreadOrder(getState()); const globalTrackOrder = initializeGlobalTrackOrder( globalTracks, null, // Passing null to urlGlobalTrackOrder to reinitilize it. - legacyThreadOrder + legacyThreadOrder, + threadActivityScores ); const localTrackOrderByPid = initializeLocalTrackOrderByPid( null, // Passing null to urlTrackOrderByPid to reinitilize it. @@ -1536,7 +1540,7 @@ export function changeTabFilter(tabID: TabID | null): ThunkAction { hiddenTracks = computeDefaultHiddenTracks( tracksWithOrder, profile, - getThreadActivityScores(getState()), + threadActivityScores, // Only include the parent process if there is no tab filter applied. includeParentProcessThreads ); diff --git a/src/profile-logic/tracks.js b/src/profile-logic/tracks.js index 597e647fc0..2dd58def12 100644 --- a/src/profile-logic/tracks.js +++ b/src/profile-logic/tracks.js @@ -167,14 +167,58 @@ function _getDefaultLocalTrackOrder(tracks: LocalTrack[], profile: ?Profile) { return trackOrder; } -function _getDefaultGlobalTrackOrder(tracks: GlobalTrack[]) { +function _getDefaultGlobalTrackOrder( + tracks: GlobalTrack[], + threadActivityScores: Array +) { const trackOrder = tracks.map((_, index) => index); + // In place sort! - trackOrder.sort( - (a, b) => - GLOBAL_TRACK_DISPLAY_ORDER[tracks[a].type] - - GLOBAL_TRACK_DISPLAY_ORDER[tracks[b].type] - ); + trackOrder.sort((a, b) => { + const trackA = tracks[a]; + const trackB = tracks[b]; + + // First, sort by track type priority (visual progress, screenshots, then process). + const typeOrderA = GLOBAL_TRACK_DISPLAY_ORDER[trackA.type]; + const typeOrderB = GLOBAL_TRACK_DISPLAY_ORDER[trackB.type]; + + if (typeOrderA !== typeOrderB) { + return typeOrderA - typeOrderB; + } + + if (trackA.type !== 'process' || trackB.type !== 'process') { + // For all the cases where both of them are not the process type, return zero. + return 0; + } + + // This is the case where both of the tracks are processes. Let's sort them + // by activity while keeping the parent process at the top. + const activityA = + trackA.mainThreadIndex !== null + ? threadActivityScores[trackA.mainThreadIndex] + : null; + const activityB = + trackB.mainThreadIndex !== null + ? threadActivityScores[trackB.mainThreadIndex] + : null; + + // Keep the parent process at the top. + if (activityA?.isInParentProcess && !activityB?.isInParentProcess) { + return -1; + } + if (!activityA?.isInParentProcess && activityB?.isInParentProcess) { + return 1; + } + + // For non-parent processes, sort by activity score. + if (activityA && activityB) { + return activityB.boostedSampleScore - activityA.boostedSampleScore; + } + + // For all other cases, maintain original order. + return 0; + }); + return trackOrder; } @@ -646,7 +690,8 @@ export function initializeGlobalTrackOrder( urlGlobalTrackOrder: TrackIndex[] | null, // If viewing an old profile URL, there were not tracks, only thread indexes. Turn // the legacy ordering into track ordering. - legacyThreadOrder: ThreadIndex[] | null + legacyThreadOrder: ThreadIndex[] | null, + threadActivityScores: Array ): TrackIndex[] { if (legacyThreadOrder !== null) { // Upgrade an older URL value based on the thread index to the track index based @@ -692,7 +737,7 @@ export function initializeGlobalTrackOrder( return urlGlobalTrackOrder !== null && _indexesAreValid(globalTracks.length, urlGlobalTrackOrder) ? urlGlobalTrackOrder - : _getDefaultGlobalTrackOrder(globalTracks); + : _getDefaultGlobalTrackOrder(globalTracks, threadActivityScores); } // Returns the selected thread (set), intersected with the set of visible threads. From f0e598339a0a36ba4240299fa52d5522de9bee35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Fri, 13 Jun 2025 12:05:16 +0200 Subject: [PATCH 2/8] Select the default selected thread by the thread activity Previously we were only getting the first tab process main thread as the selected thread, which wasn't the track we want most of the time. --- src/actions/receive-profile.js | 6 ++-- src/profile-logic/profile-data.js | 14 ++++++-- src/profile-logic/tracks.js | 47 ++++++++++++++++++++------ src/test/store/receive-profile.test.js | 4 +-- 4 files changed, 53 insertions(+), 18 deletions(-) diff --git a/src/actions/receive-profile.js b/src/actions/receive-profile.js index d397cab165..87d722e7be 100644 --- a/src/actions/receive-profile.js +++ b/src/actions/receive-profile.js @@ -319,7 +319,8 @@ export function finalizeFullProfileView( const selectedThreadIndexes = initializeSelectedThreadIndex( maybeSelectedThreadIndexes, getVisibleThreads(tracksWithOrder, hiddenTracks), - profile + profile, + threadActivityScores ); let timelineType = null; @@ -1549,7 +1550,8 @@ export function changeTabFilter(tabID: TabID | null): ThunkAction { const selectedThreadIndexes = initializeSelectedThreadIndex( null, // maybeSelectedThreadIndexes getVisibleThreads(tracksWithOrder, hiddenTracks), - profile + profile, + threadActivityScores ); // If the currently selected tab is only visible when the selected track diff --git a/src/profile-logic/profile-data.js b/src/profile-logic/profile-data.js index 1aa5de9d60..e537dcbdde 100644 --- a/src/profile-logic/profile-data.js +++ b/src/profile-logic/profile-data.js @@ -95,6 +95,7 @@ import type { TabID, } from 'firefox-profiler/types'; import type { CallNodeInfo, SuffixOrderIndex } from './call-node-info'; +import type { ThreadActivityScore } from './tracks'; /** * Various helpers for dealing with the profile as a data structure. @@ -1286,8 +1287,12 @@ export function getTimeRangeIncludingAllThreads( return completeRange; } -export function defaultThreadOrder(threads: RawThread[]): ThreadIndex[] { - const threadOrder = threads.map((thread, i) => i); +export function defaultThreadOrder( + visibleThreadIndexes: ThreadIndex[], + threads: RawThread[], + threadActivityScores: Array +): ThreadIndex[] { + const threadOrder = [...visibleThreadIndexes]; // Note: to have a consistent behavior independant of the sorting algorithm, // we need to be careful that the comparator function is consistent: @@ -1299,7 +1304,10 @@ export function defaultThreadOrder(threads: RawThread[]): ThreadIndex[] { const nameB = threads[b].name; if (nameA === nameB) { - return a - b; + return ( + threadActivityScores[b].boostedSampleScore - + threadActivityScores[a].boostedSampleScore + ); } // Put the compositor/renderer thread last. diff --git a/src/profile-logic/tracks.js b/src/profile-logic/tracks.js index 2dd58def12..fb0896d03e 100644 --- a/src/profile-logic/tracks.js +++ b/src/profile-logic/tracks.js @@ -745,10 +745,15 @@ export function initializeGlobalTrackOrder( export function initializeSelectedThreadIndex( selectedThreadIndexes: Set | null, visibleThreadIndexes: ThreadIndex[], - profile: Profile + profile: Profile, + threadActivityScores: Array ): Set { if (selectedThreadIndexes === null) { - return getDefaultSelectedThreadIndexes(visibleThreadIndexes, profile); + return getDefaultSelectedThreadIndexes( + visibleThreadIndexes, + profile, + threadActivityScores + ); } // Filter out hidden threads from the set of selected threads. @@ -758,7 +763,11 @@ export function initializeSelectedThreadIndex( ); if (visibleSelectedThreadIndexes.size === 0) { // No selected threads were visible. Fall back to default selection. - return getDefaultSelectedThreadIndexes(visibleThreadIndexes, profile); + return getDefaultSelectedThreadIndexes( + visibleThreadIndexes, + profile, + threadActivityScores + ); } return visibleSelectedThreadIndexes; } @@ -767,7 +776,8 @@ export function initializeSelectedThreadIndex( // order. function getDefaultSelectedThreadIndexes( visibleThreadIndexes: ThreadIndex[], - profile: Profile + profile: Profile, + threadActivityScores: Array ): Set { if (profile.meta.initialSelectedThreads !== undefined) { return new Set( @@ -785,10 +795,11 @@ function getDefaultSelectedThreadIndexes( }) ); } - const visibleThreads = visibleThreadIndexes.map( - (threadIndex) => profile.threads[threadIndex] + const defaultThread = _findDefaultThread( + visibleThreadIndexes, + profile.threads, + threadActivityScores ); - const defaultThread = _findDefaultThread(visibleThreads); const defaultThreadIndex = profile.threads.indexOf(defaultThread); if (defaultThreadIndex === -1) { throw new Error('Expected to find a thread index to select.'); @@ -1309,16 +1320,30 @@ function _computeThreadSampleScore( return nonIdleSampleCount * referenceCPUDeltaPerInterval; } -function _findDefaultThread(threads: RawThread[]): RawThread | null { +function _findDefaultThread( + visibleThreadIndexes: ThreadIndex[], + threads: RawThread[], + threadActivityScores: Array +): RawThread | null { if (threads.length === 0) { // Tests may have no threads. return null; } - const contentThreadId = threads.findIndex( - (thread) => thread.name === 'GeckoMain' && thread.processType === 'tab' + + const threadOrder = defaultThreadOrder( + visibleThreadIndexes, + threads, + threadActivityScores ); + + // Try to find a tab process with the highest activity score. If it can't + // find one, select the first thread with the highest one. const defaultThreadIndex = - contentThreadId !== -1 ? contentThreadId : defaultThreadOrder(threads)[0]; + threadOrder.find( + (threadIndex) => + threads[threadIndex].name === 'GeckoMain' && + threads[threadIndex].processType === 'tab' + ) ?? threadOrder[0]; return threads[defaultThreadIndex]; } diff --git a/src/test/store/receive-profile.test.js b/src/test/store/receive-profile.test.js index 36fa4cccf1..a08457b6e8 100644 --- a/src/test/store/receive-profile.test.js +++ b/src/test/store/receive-profile.test.js @@ -265,9 +265,9 @@ describe('actions/receive-profile', function () { store.dispatch(viewProfile(profile)); expect(getHumanReadableTracks(store.getState())).toEqual([ - 'hide [thread GeckoMain tab]', 'show [thread GeckoMain tab] SELECTED', 'hide [thread GeckoMain tab]', + 'hide [thread GeckoMain tab]', ]); }); @@ -396,8 +396,8 @@ describe('actions/receive-profile', function () { store.dispatch(viewProfile(profile)); expect(getHumanReadableTracks(store.getState())).toEqual([ - 'hide [thread GeckoMain tab]', 'show [thread GeckoMain tab] SELECTED', + 'hide [thread GeckoMain tab]', ]); }); From 67e1f00159746a0214709a8cd1a5e6dee76408d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Sat, 14 Jun 2025 21:23:31 +0200 Subject: [PATCH 3/8] Add tests for ordering and selecting the tracks by activity --- src/test/store/tracks.test.js | 198 ++++++++++++++++++++++++++++++++++ 1 file changed, 198 insertions(+) diff --git a/src/test/store/tracks.test.js b/src/test/store/tracks.test.js index 10e6f499c0..8da6b55bca 100644 --- a/src/test/store/tracks.test.js +++ b/src/test/store/tracks.test.js @@ -7,6 +7,8 @@ import { getNetworkTrackProfile, addIPCMarkerPairToThreads, getProfileWithMarkers, + getProfileFromTextSamples, + getProfileWithThreadCPUDelta, } from '../fixtures/profiles/processed-profile'; import { getEmptyThread } from '../../profile-logic/data-structures'; import { storeWithProfile } from '../fixtures/stores'; @@ -396,6 +398,202 @@ describe('ordering and hiding', function () { ).toEqual(userFacingSortOrder); }); }); + + describe('ordering by activity', function () { + it('orders process tracks by activity score while keeping parent process first', function () { + const { profile } = getProfileFromTextSamples( + 'A B C D E F G H I J', // High activity parent process + 'X Y', // Low activity tab process + 'P Q R S T U V W X Y Z A B C D' // Very high activity tab process + ); + + // Set up threads as different processes + profile.threads[0].name = 'GeckoMain'; + profile.threads[0].isMainThread = true; + profile.threads[0].processType = 'default'; // Parent process + profile.threads[0].pid = '1'; + + profile.threads[1].name = 'GeckoMain'; + profile.threads[1].isMainThread = true; + profile.threads[1].processType = 'tab'; + profile.threads[1].pid = '2'; + + profile.threads[2].name = 'GeckoMain'; + profile.threads[2].isMainThread = true; + profile.threads[2].processType = 'tab'; + profile.threads[2].pid = '3'; + + const { getState } = storeWithProfile(profile); + + // Parent process should be first, then tab processes ordered by activity + // The highest activity tab process (index 2) should be selected by default + expect(getHumanReadableTracks(getState())).toEqual([ + 'show [thread GeckoMain default]', // Parent process first + 'show [thread GeckoMain tab] SELECTED', // Very high activity tab process selected + 'show [thread GeckoMain tab]', // Low activity tab process + ]); + + const globalTrackOrder = + UrlStateSelectors.getGlobalTrackOrder(getState()); + expect(globalTrackOrder).toEqual([0, 2, 1]); + }); + + it('orders multiple tab processes by their activity levels', function () { + const profile = getProfileWithThreadCPUDelta([ + [5, 5, 5], // Low activity tab process + [50, 50, 50], // High activity tab process + [25, 25, 25], // Medium activity tab process + ]); + + profile.threads[0].name = 'GeckoMain'; + profile.threads[0].isMainThread = true; + profile.threads[0].processType = 'tab'; + profile.threads[0].pid = '1'; + + profile.threads[1].name = 'GeckoMain'; + profile.threads[1].isMainThread = true; + profile.threads[1].processType = 'tab'; + profile.threads[1].pid = '2'; + + profile.threads[2].name = 'GeckoMain'; + profile.threads[2].isMainThread = true; + profile.threads[2].processType = 'tab'; + profile.threads[2].pid = '3'; + + const { getState } = storeWithProfile(profile); + + // Processes should be ordered by activity: high, medium, low + expect(getHumanReadableTracks(getState())).toEqual([ + 'show [thread GeckoMain tab] SELECTED', // High activity (index 1) + 'show [thread GeckoMain tab]', // Medium activity (index 2) + 'show [thread GeckoMain tab]', // Low activity (index 0) + ]); + + const globalTrackOrder = + UrlStateSelectors.getGlobalTrackOrder(getState()); + expect(globalTrackOrder).toEqual([1, 2, 0]); + }); + + it('handles processes without main threads gracefully', function () { + const { profile } = getProfileFromTextSamples('A', 'B'); + + profile.threads[0].name = 'GeckoMain'; + profile.threads[0].isMainThread = true; + profile.threads[0].processType = 'tab'; + profile.threads[0].pid = '1'; + + profile.threads[1].name = 'DOM Worker'; + profile.threads[1].isMainThread = false; + profile.threads[1].processType = 'tab'; + profile.threads[1].pid = '2'; // Different PID, no main thread + + const { getState } = storeWithProfile(profile); + + // Should not crash and maintain a stable order + expect(getHumanReadableTracks(getState())).toEqual([ + 'show [thread GeckoMain tab] SELECTED', + 'show [process]', + ' - show [thread DOM Worker]', + ]); + + const globalTrackOrder = + UrlStateSelectors.getGlobalTrackOrder(getState()); + expect(globalTrackOrder).toEqual([0, 1]); + }); + }); + + describe('default selected thread selection by activity', function () { + it('selects the tab process with highest activity as default', function () { + const profile = getProfileWithThreadCPUDelta([ + [10, 10, 10], // Low activity parent process + [5, 5, 5], // Low activity tab process + [50, 50, 50], // High activity tab process + ]); + + profile.threads[0].name = 'GeckoMain'; + profile.threads[0].isMainThread = true; + profile.threads[0].processType = 'default'; // Parent process + profile.threads[0].pid = '0'; + + profile.threads[1].name = 'GeckoMain'; + profile.threads[1].isMainThread = true; + profile.threads[1].processType = 'tab'; + profile.threads[1].pid = '1'; + + profile.threads[2].name = 'GeckoMain'; + profile.threads[2].isMainThread = true; + profile.threads[2].processType = 'tab'; + profile.threads[2].pid = '2'; + + const { getState } = storeWithProfile(profile); + + // The high activity tab process (index 2) should be selected + expect(getHumanReadableTracks(getState())).toEqual([ + 'show [thread GeckoMain default]', // Parent process + 'show [thread GeckoMain tab] SELECTED', // High activity tab selected + 'show [thread GeckoMain tab]', // Low activity tab + ]); + + const globalTrackOrder = + UrlStateSelectors.getGlobalTrackOrder(getState()); + expect(globalTrackOrder).toEqual([0, 2, 1]); + }); + + it('falls back to first thread when no tab processes exist', function () { + const profile = getProfileWithThreadCPUDelta([ + [10, 10, 10], // Parent process only + ]); + + profile.threads[0].name = 'GeckoMain'; + profile.threads[0].isMainThread = true; + profile.threads[0].processType = 'default'; + profile.threads[0].pid = '0'; + + const { getState } = storeWithProfile(profile); + + // Parent process should be selected as fallback + expect(getHumanReadableTracks(getState())).toEqual([ + 'show [thread GeckoMain default] SELECTED', + ]); + }); + + it('selects highest activity tab even when parent has higher activity', function () { + const profile = getProfileWithThreadCPUDelta([ + [100, 100, 100], // Very high activity parent process + [50, 50, 50], // Medium activity tab process + [75, 75, 75], // High activity tab process + ]); + + profile.threads[0].name = 'GeckoMain'; + profile.threads[0].isMainThread = true; + profile.threads[0].processType = 'default'; // Parent process + profile.threads[0].pid = '0'; + + profile.threads[1].name = 'GeckoMain'; + profile.threads[1].isMainThread = true; + profile.threads[1].processType = 'tab'; + profile.threads[1].pid = '1'; + + profile.threads[2].name = 'GeckoMain'; + profile.threads[2].isMainThread = true; + profile.threads[2].processType = 'tab'; + profile.threads[2].pid = '2'; + + const { getState } = storeWithProfile(profile); + + // The highest activity tab process (index 2) should be selected, + // not the parent process despite it having higher activity + expect(getHumanReadableTracks(getState())).toEqual([ + 'show [thread GeckoMain default]', // Parent process not selected + 'show [thread GeckoMain tab] SELECTED', // High activity tab selected + 'show [thread GeckoMain tab]', // Medium activity tab + ]); + + const globalTrackOrder = + UrlStateSelectors.getGlobalTrackOrder(getState()); + expect(globalTrackOrder).toEqual([0, 2, 1]); + }); + }); }); describe('local tracks', function () { From 8c5866f3acbe236fda436dc0bccc2c73e9dab690 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Thu, 26 Jun 2025 23:05:04 +0200 Subject: [PATCH 4/8] Inline the _findDefaultThread function inside getDefaultSelectedThreadIndexes --- src/profile-logic/tracks.js | 52 +++++++++++++------------------------ 1 file changed, 18 insertions(+), 34 deletions(-) diff --git a/src/profile-logic/tracks.js b/src/profile-logic/tracks.js index fb0896d03e..55a3857e3a 100644 --- a/src/profile-logic/tracks.js +++ b/src/profile-logic/tracks.js @@ -795,15 +795,27 @@ function getDefaultSelectedThreadIndexes( }) ); } - const defaultThread = _findDefaultThread( + + const { threads } = profile; + if (threads.length === 0) { + throw new Error('Expected to find a thread index to select.'); + } + + const threadOrder = defaultThreadOrder( visibleThreadIndexes, - profile.threads, + threads, threadActivityScores ); - const defaultThreadIndex = profile.threads.indexOf(defaultThread); - if (defaultThreadIndex === -1) { - throw new Error('Expected to find a thread index to select.'); - } + + // Try to find a tab process with the highest activity score. If it can't + // find one, select the first thread with the highest one. + const defaultThreadIndex = + threadOrder.find( + (threadIndex) => + threads[threadIndex].name === 'GeckoMain' && + threads[threadIndex].processType === 'tab' + ) ?? threadOrder[0]; + return new Set([defaultThreadIndex]); } @@ -1320,34 +1332,6 @@ function _computeThreadSampleScore( return nonIdleSampleCount * referenceCPUDeltaPerInterval; } -function _findDefaultThread( - visibleThreadIndexes: ThreadIndex[], - threads: RawThread[], - threadActivityScores: Array -): RawThread | null { - if (threads.length === 0) { - // Tests may have no threads. - return null; - } - - const threadOrder = defaultThreadOrder( - visibleThreadIndexes, - threads, - threadActivityScores - ); - - // Try to find a tab process with the highest activity score. If it can't - // find one, select the first thread with the highest one. - const defaultThreadIndex = - threadOrder.find( - (threadIndex) => - threads[threadIndex].name === 'GeckoMain' && - threads[threadIndex].processType === 'tab' - ) ?? threadOrder[0]; - - return threads[defaultThreadIndex]; -} - function _indexesAreValid(listLength: number, indexes: number[]) { return ( // The item length is valid. From cb71088c9266880032cf04705d59df54ff518dec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Thu, 26 Jun 2025 23:12:30 +0200 Subject: [PATCH 5/8] Move defaultThreadOrder to tracks.js --- src/profile-logic/profile-data.js | 49 ----------------------------- src/profile-logic/tracks.js | 51 +++++++++++++++++++++++++++++-- 2 files changed, 49 insertions(+), 51 deletions(-) diff --git a/src/profile-logic/profile-data.js b/src/profile-logic/profile-data.js index e537dcbdde..bbba49027a 100644 --- a/src/profile-logic/profile-data.js +++ b/src/profile-logic/profile-data.js @@ -95,7 +95,6 @@ import type { TabID, } from 'firefox-profiler/types'; import type { CallNodeInfo, SuffixOrderIndex } from './call-node-info'; -import type { ThreadActivityScore } from './tracks'; /** * Various helpers for dealing with the profile as a data structure. @@ -1287,54 +1286,6 @@ export function getTimeRangeIncludingAllThreads( return completeRange; } -export function defaultThreadOrder( - visibleThreadIndexes: ThreadIndex[], - threads: RawThread[], - threadActivityScores: Array -): ThreadIndex[] { - const threadOrder = [...visibleThreadIndexes]; - - // Note: to have a consistent behavior independant of the sorting algorithm, - // we need to be careful that the comparator function is consistent: - // comparator(a, b) === - comparator(b, a) - // and - // comparator(a, b) === 0 if and only if a === b - threadOrder.sort((a, b) => { - const nameA = threads[a].name; - const nameB = threads[b].name; - - if (nameA === nameB) { - return ( - threadActivityScores[b].boostedSampleScore - - threadActivityScores[a].boostedSampleScore - ); - } - - // Put the compositor/renderer thread last. - // Compositor will always be before Renderer, if both are present. - if (nameA === 'Compositor') { - return 1; - } - - if (nameB === 'Compositor') { - return -1; - } - - if (nameA === 'Renderer') { - return 1; - } - - if (nameB === 'Renderer') { - return -1; - } - - // Otherwise keep the existing order. We don't return 0 to guarantee that - // the sort is stable even if the sort algorithm isn't. - return a - b; - }); - return threadOrder; -} - export function toValidImplementationFilter( implementation: string ): ImplementationFilter { diff --git a/src/profile-logic/tracks.js b/src/profile-logic/tracks.js index 55a3857e3a..79c3deec85 100644 --- a/src/profile-logic/tracks.js +++ b/src/profile-logic/tracks.js @@ -20,7 +20,6 @@ import type { } from 'firefox-profiler/types'; import { - defaultThreadOrder, getFriendlyThreadName, computeStackTableFromRawStackTable, } from './profile-data'; @@ -801,7 +800,7 @@ function getDefaultSelectedThreadIndexes( throw new Error('Expected to find a thread index to select.'); } - const threadOrder = defaultThreadOrder( + const threadOrder = _defaultThreadOrder( visibleThreadIndexes, threads, threadActivityScores @@ -819,6 +818,54 @@ function getDefaultSelectedThreadIndexes( return new Set([defaultThreadIndex]); } +function _defaultThreadOrder( + visibleThreadIndexes: ThreadIndex[], + threads: RawThread[], + threadActivityScores: Array +): ThreadIndex[] { + const threadOrder = [...visibleThreadIndexes]; + + // Note: to have a consistent behavior independant of the sorting algorithm, + // we need to be careful that the comparator function is consistent: + // comparator(a, b) === - comparator(b, a) + // and + // comparator(a, b) === 0 if and only if a === b + threadOrder.sort((a, b) => { + const nameA = threads[a].name; + const nameB = threads[b].name; + + if (nameA === nameB) { + return ( + threadActivityScores[b].boostedSampleScore - + threadActivityScores[a].boostedSampleScore + ); + } + + // Put the compositor/renderer thread last. + // Compositor will always be before Renderer, if both are present. + if (nameA === 'Compositor') { + return 1; + } + + if (nameB === 'Compositor') { + return -1; + } + + if (nameA === 'Renderer') { + return 1; + } + + if (nameB === 'Renderer') { + return -1; + } + + // Otherwise keep the existing order. We don't return 0 to guarantee that + // the sort is stable even if the sort algorithm isn't. + return a - b; + }); + return threadOrder; +} + // Returns either a configuration of hidden tracks that has at least one // visible thread, or null. export function tryInitializeHiddenTracksLegacy( From 5e580a618ed57098ea71e5caf15c83b0df87c7dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Fri, 27 Jun 2025 11:27:19 +0200 Subject: [PATCH 6/8] Sort the threads that have different name too and fall back to original sorting behavior This created some test changes how we find the selected thread in comparison profiles especially. But after thinking about it, I decided that the new behavior is actually better for them. Previously we were always getting the first track, but now we are getting the crowded one, which is usually the diffing track. --- src/profile-logic/tracks.js | 9 +++++++-- src/test/store/profile-view.test.js | 5 +++++ src/test/store/receive-profile.test.js | 20 ++++++++++---------- src/test/store/useful-tabs.test.js | 6 ++++++ 4 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/profile-logic/tracks.js b/src/profile-logic/tracks.js index 79c3deec85..604bbe1522 100644 --- a/src/profile-logic/tracks.js +++ b/src/profile-logic/tracks.js @@ -835,9 +835,11 @@ function _defaultThreadOrder( const nameB = threads[b].name; if (nameA === nameB) { + // Sort by the activity, but keep the original order if the activity + // scores are the equal. return ( threadActivityScores[b].boostedSampleScore - - threadActivityScores[a].boostedSampleScore + threadActivityScores[a].boostedSampleScore || a - b ); } @@ -861,7 +863,10 @@ function _defaultThreadOrder( // Otherwise keep the existing order. We don't return 0 to guarantee that // the sort is stable even if the sort algorithm isn't. - return a - b; + return ( + threadActivityScores[b].boostedSampleScore - + threadActivityScores[a].boostedSampleScore || a - b + ); }); return threadOrder; } diff --git a/src/test/store/profile-view.test.js b/src/test/store/profile-view.test.js index 64b014760b..89745d425a 100644 --- a/src/test/store/profile-view.test.js +++ b/src/test/store/profile-view.test.js @@ -534,6 +534,10 @@ describe('actions/ProfileView', function () { describe('with a comparison profile', function () { it('selects the calltree tab when selecting the diffing track', function () { + const firstTrackReference = { + type: 'global', + trackIndex: 0, + }; const diffingTrackReference = { type: 'global', trackIndex: 2, @@ -545,6 +549,7 @@ describe('actions/ProfileView', function () { ]); const { getState, dispatch } = storeWithProfile(profile); + dispatch(ProfileView.selectTrackWithModifiers(firstTrackReference)); dispatch(App.changeSelectedTab('flame-graph')); expect(UrlStateSelectors.getSelectedThreadIndexes(getState())).toEqual( new Set([0]) diff --git a/src/test/store/receive-profile.test.js b/src/test/store/receive-profile.test.js index a08457b6e8..8e6ad94616 100644 --- a/src/test/store/receive-profile.test.js +++ b/src/test/store/receive-profile.test.js @@ -411,9 +411,9 @@ describe('actions/receive-profile', function () { 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]', + 'show [thread Empty default]', + 'show [thread Diff between 1 and 2 comparison] SELECTED', ]); }); @@ -514,9 +514,9 @@ describe('actions/receive-profile', function () { 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]', + 'show [thread Empty default]', + 'show [thread Diff between 1 and 2 comparison] SELECTED', ]); }); }); @@ -571,7 +571,7 @@ describe('actions/receive-profile', function () { ' - show [thread Thread with 140 CPU]', ' - show [thread Thread with 160 CPU]', ' - show [thread Thread with 180 CPU]', - ' - show [thread Thread with 190 CPU] SELECTED', + ' - show [thread Thread with 190 CPU]', ' - show [thread Thread with 220 CPU]', ' - show [thread Thread with 230 CPU]', ' - show [thread Thread with 270 CPU]', @@ -579,7 +579,7 @@ describe('actions/receive-profile', function () { ' - show [thread Thread with 320 CPU]', ' - show [thread Thread with 330 CPU]', ' - show [thread Thread with 350 CPU]', - ' - show [thread Thread with 380 CPU]', + ' - show [thread Thread with 380 CPU] SELECTED', ]); }); @@ -634,7 +634,7 @@ describe('actions/receive-profile', function () { ' - show [thread Thread with 130 CPU]', ' - show [thread Thread with 140 CPU]', ' - show [thread Thread with 180 CPU]', - ' - show [thread Thread with 190 CPU] SELECTED', + ' - show [thread Thread with 190 CPU]', ' - show [thread Thread with 220 CPU]', ' - show [thread Thread with 230 CPU]', ' - show [thread Thread with 270 CPU]', @@ -642,7 +642,7 @@ describe('actions/receive-profile', function () { ' - show [thread Thread with 320 CPU]', ' - show [thread Thread with 330 CPU]', ' - show [thread Thread with 350 CPU]', - ' - show [thread Thread with 380 CPU]', + ' - show [thread Thread with 380 CPU] SELECTED', ]); }); }); @@ -1899,9 +1899,9 @@ describe('actions/receive-profile', function () { store.dispatch(viewProfile(resultProfile)); expect(getHumanReadableTracks(store.getState())).toEqual([ - 'show [thread Empty default] SELECTED', 'show [thread Empty default]', - 'show [thread Diff between 1 and 2 comparison]', + 'show [thread Empty default]', + 'show [thread Diff between 1 and 2 comparison] SELECTED', ]); }); diff --git a/src/test/store/useful-tabs.test.js b/src/test/store/useful-tabs.test.js index f0614bf790..c7512bf53a 100644 --- a/src/test/store/useful-tabs.test.js +++ b/src/test/store/useful-tabs.test.js @@ -53,6 +53,12 @@ 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 { getState, dispatch } = storeWithProfile(profile); + dispatch({ + type: 'SELECT_TRACK', + selectedThreadIndexes: new Set([0]), + selectedTab: 'calltree', + lastNonShiftClickInformation: null, + }); expect(selectedThreadSelectors.getUsefulTabs(getState())).toEqual([ 'calltree', 'flame-graph', From a2afd375032c303510832cc87645a7696e187b17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Fri, 27 Jun 2025 11:48:36 +0200 Subject: [PATCH 7/8] Update the old comment --- src/profile-logic/tracks.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/profile-logic/tracks.js b/src/profile-logic/tracks.js index 604bbe1522..863e7a320d 100644 --- a/src/profile-logic/tracks.js +++ b/src/profile-logic/tracks.js @@ -771,8 +771,11 @@ export function initializeSelectedThreadIndex( return visibleSelectedThreadIndexes; } -// Select either the GeckoMain [tab] thread, or the first thread in the thread -// order. +// Select either the most active GeckoMain [tab] thread, or the most active +// thread sorted by the thread activity scores. +// It always selects global tracks when there is a GeckoMain [tab], but when +// there is no GeckoMain [tab], it might select local tracks too depending +// on the activity score. function getDefaultSelectedThreadIndexes( visibleThreadIndexes: ThreadIndex[], profile: Profile, From fc1a8d3b40d74cb42fa9808943817c1d9d4832ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Fri, 27 Jun 2025 11:53:13 +0200 Subject: [PATCH 8/8] Explain when mainThreadIndex might be null --- src/profile-logic/tracks.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/profile-logic/tracks.js b/src/profile-logic/tracks.js index 863e7a320d..2641fa0849 100644 --- a/src/profile-logic/tracks.js +++ b/src/profile-logic/tracks.js @@ -192,6 +192,8 @@ function _getDefaultGlobalTrackOrder( // This is the case where both of the tracks are processes. Let's sort them // by activity while keeping the parent process at the top. + // mainThreadIndex might be null in case the GeckoMain thread is not + // profiled in a profile. const activityA = trackA.mainThreadIndex !== null ? threadActivityScores[trackA.mainThreadIndex]