Order global tracks by activity and select the most active non-parent process by default (#5491)

Previously we were keeping the original order in the profile data
structure when we were initializing the global track order, and
selecting the first tab visible process by default. But this was mostly
incorrect for the cases where we have more than one visible tab process,
because usually busier thread arrives later during the profile
collection.

This should help our users to see the most active process very quickly
instead of having to scroll down every time.

Author: canova

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
       );
@@ -317,7 +319,8 @@ export function finalizeFullProfileView(
     const selectedThreadIndexes = initializeSelectedThreadIndex(
       maybeSelectedThreadIndexes,
       getVisibleThreads(tracksWithOrder, hiddenTracks),
-      profile
+      profile,
+      threadActivityScores
     );
 
     let timelineType = null;
@@ -1495,11 +1498,13 @@ export function changeTabFilter(tabID: TabID | null): ThunkAction<void> {
     );
     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 +1541,7 @@ export function changeTabFilter(tabID: TabID | null): ThunkAction<void> {
       hiddenTracks = computeDefaultHiddenTracks(
         tracksWithOrder,
         profile,
-        getThreadActivityScores(getState()),
+        threadActivityScores,
         // Only include the parent process if there is no tab filter applied.
         includeParentProcessThreads
       );
@@ -1545,7 +1550,8 @@ export function changeTabFilter(tabID: TabID | null): ThunkAction<void> {
     const selectedThreadIndexes = initializeSelectedThreadIndex(
       null, // maybeSelectedThreadIndexes
       getVisibleThreads(tracksWithOrder, hiddenTracks),
-      profile
+      profile,
+      threadActivityScores
     );
 
     // If the currently selected tab is only visible when the selected track

src/profile-logic/profile-data.js

@@ -1286,47 +1286,6 @@ export function getTimeRangeIncludingAllThreads(
   return completeRange;
 }
 
-export function defaultThreadOrder(threads: RawThread[]): ThreadIndex[] {
-  const threadOrder = threads.map((thread, i) => i);
-
-  // 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 a - b;
-    }
-
-    // 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 {

src/profile-logic/tracks.js

@@ -20,7 +20,6 @@ import type {
 } from 'firefox-profiler/types';
 
 import {
-  defaultThreadOrder,
   getFriendlyThreadName,
   computeStackTableFromRawStackTable,
 } from './profile-data';
@@ -167,14 +166,60 @@ function _getDefaultLocalTrackOrder(tracks: LocalTrack[], profile: ?Profile) {
   return trackOrder;
 }
 
-function _getDefaultGlobalTrackOrder(tracks: GlobalTrack[]) {
+function _getDefaultGlobalTrackOrder(
+  tracks: GlobalTrack[],
+  threadActivityScores: Array<ThreadActivityScore>
+) {
   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.
+    // mainThreadIndex might be null in case the GeckoMain thread is not
+    // profiled in a profile.
+    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 +691,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<ThreadActivityScore>
 ): TrackIndex[] {
   if (legacyThreadOrder !== null) {
     // Upgrade an older URL value based on the thread index to the track index based
@@ -692,18 +738,23 @@ 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.
 // Falls back to the default thread selection.
 export function initializeSelectedThreadIndex(
   selectedThreadIndexes: Set<ThreadIndex> | null,
   visibleThreadIndexes: ThreadIndex[],
-  profile: Profile
+  profile: Profile,
+  threadActivityScores: Array<ThreadActivityScore>
 ): Set<ThreadIndex> {
   if (selectedThreadIndexes === null) {
-    return getDefaultSelectedThreadIndexes(visibleThreadIndexes, profile);
+    return getDefaultSelectedThreadIndexes(
+      visibleThreadIndexes,
+      profile,
+      threadActivityScores
+    );
   }
 
   // Filter out hidden threads from the set of selected threads.
@@ -713,16 +764,24 @@ 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;
 }
 
-// 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
+  profile: Profile,
+  threadActivityScores: Array<ThreadActivityScore>
 ): Set<ThreadIndex> {
   if (profile.meta.initialSelectedThreads !== undefined) {
     return new Set(
@@ -740,17 +799,83 @@ function getDefaultSelectedThreadIndexes(
       })
     );
   }
-  const visibleThreads = visibleThreadIndexes.map(
-    (threadIndex) => profile.threads[threadIndex]
-  );
-  const defaultThread = _findDefaultThread(visibleThreads);
-  const defaultThreadIndex = profile.threads.indexOf(defaultThread);
-  if (defaultThreadIndex === -1) {
+
+  const { threads } = profile;
+  if (threads.length === 0) {
     throw new Error('Expected to find a thread index to select.');
   }
+
+  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 new Set([defaultThreadIndex]);
 }
 
+function _defaultThreadOrder(
+  visibleThreadIndexes: ThreadIndex[],
+  threads: RawThread[],
+  threadActivityScores: Array<ThreadActivityScore>
+): 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) {
+      // Sort by the activity, but keep the original order if the activity
+      // scores are the equal.
+      return (
+        threadActivityScores[b].boostedSampleScore -
+          threadActivityScores[a].boostedSampleScore || a - b
+      );
+    }
+
+    // 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 (
+      threadActivityScores[b].boostedSampleScore -
+        threadActivityScores[a].boostedSampleScore || a - b
+    );
+  });
+  return threadOrder;
+}
+
 // Returns either a configuration of hidden tracks that has at least one
 // visible thread, or null.
 export function tryInitializeHiddenTracksLegacy(
@@ -1264,20 +1389,6 @@ function _computeThreadSampleScore(
   return nonIdleSampleCount * referenceCPUDeltaPerInterval;
 }
 
-function _findDefaultThread(threads: RawThread[]): 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 defaultThreadIndex =
-    contentThreadId !== -1 ? contentThreadId : defaultThreadOrder(threads)[0];
-
-  return threads[defaultThreadIndex];
-}
-
 function _indexesAreValid(listLength: number, indexes: number[]) {
   return (
     // The item length is valid.

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])