Skip to content

Commit 5e580a6

Browse files
committed
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.
1 parent cb71088 commit 5e580a6

File tree

4 files changed

+28
-12
lines changed

4 files changed

+28
-12
lines changed

src/profile-logic/tracks.js

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -835,9 +835,11 @@ function _defaultThreadOrder(
835835
const nameB = threads[b].name;
836836

837837
if (nameA === nameB) {
838+
// Sort by the activity, but keep the original order if the activity
839+
// scores are the equal.
838840
return (
839841
threadActivityScores[b].boostedSampleScore -
840-
threadActivityScores[a].boostedSampleScore
842+
threadActivityScores[a].boostedSampleScore || a - b
841843
);
842844
}
843845

@@ -861,7 +863,10 @@ function _defaultThreadOrder(
861863

862864
// Otherwise keep the existing order. We don't return 0 to guarantee that
863865
// the sort is stable even if the sort algorithm isn't.
864-
return a - b;
866+
return (
867+
threadActivityScores[b].boostedSampleScore -
868+
threadActivityScores[a].boostedSampleScore || a - b
869+
);
865870
});
866871
return threadOrder;
867872
}

src/test/store/profile-view.test.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,10 @@ describe('actions/ProfileView', function () {
534534

535535
describe('with a comparison profile', function () {
536536
it('selects the calltree tab when selecting the diffing track', function () {
537+
const firstTrackReference = {
538+
type: 'global',
539+
trackIndex: 0,
540+
};
537541
const diffingTrackReference = {
538542
type: 'global',
539543
trackIndex: 2,
@@ -545,6 +549,7 @@ describe('actions/ProfileView', function () {
545549
]);
546550
const { getState, dispatch } = storeWithProfile(profile);
547551

552+
dispatch(ProfileView.selectTrackWithModifiers(firstTrackReference));
548553
dispatch(App.changeSelectedTab('flame-graph'));
549554
expect(UrlStateSelectors.getSelectedThreadIndexes(getState())).toEqual(
550555
new Set([0])

src/test/store/receive-profile.test.js

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -411,9 +411,9 @@ describe('actions/receive-profile', function () {
411411

412412
store.dispatch(viewProfile(profile));
413413
expect(getHumanReadableTracks(store.getState())).toEqual([
414-
'show [thread Empty default] SELECTED',
415414
'show [thread Empty default]',
416-
'show [thread Diff between 1 and 2 comparison]',
415+
'show [thread Empty default]',
416+
'show [thread Diff between 1 and 2 comparison] SELECTED',
417417
]);
418418
});
419419

@@ -514,9 +514,9 @@ describe('actions/receive-profile', function () {
514514

515515
store.dispatch(viewProfile(profile));
516516
expect(getHumanReadableTracks(store.getState())).toEqual([
517-
'show [thread Empty default] SELECTED',
518517
'show [thread Empty default]',
519-
'show [thread Diff between 1 and 2 comparison]',
518+
'show [thread Empty default]',
519+
'show [thread Diff between 1 and 2 comparison] SELECTED',
520520
]);
521521
});
522522
});
@@ -571,15 +571,15 @@ describe('actions/receive-profile', function () {
571571
' - show [thread Thread with 140 CPU]',
572572
' - show [thread Thread with 160 CPU]',
573573
' - show [thread Thread with 180 CPU]',
574-
' - show [thread Thread with 190 CPU] SELECTED',
574+
' - show [thread Thread with 190 CPU]',
575575
' - show [thread Thread with 220 CPU]',
576576
' - show [thread Thread with 230 CPU]',
577577
' - show [thread Thread with 270 CPU]',
578578
' - show [thread Thread with 310 CPU]',
579579
' - show [thread Thread with 320 CPU]',
580580
' - show [thread Thread with 330 CPU]',
581581
' - show [thread Thread with 350 CPU]',
582-
' - show [thread Thread with 380 CPU]',
582+
' - show [thread Thread with 380 CPU] SELECTED',
583583
]);
584584
});
585585

@@ -634,15 +634,15 @@ describe('actions/receive-profile', function () {
634634
' - show [thread Thread with 130 CPU]',
635635
' - show [thread Thread with 140 CPU]',
636636
' - show [thread Thread with 180 CPU]',
637-
' - show [thread Thread with 190 CPU] SELECTED',
637+
' - show [thread Thread with 190 CPU]',
638638
' - show [thread Thread with 220 CPU]',
639639
' - show [thread Thread with 230 CPU]',
640640
' - show [thread Thread with 270 CPU]',
641641
' - show [thread Thread with 310 CPU]',
642642
' - show [thread Thread with 320 CPU]',
643643
' - show [thread Thread with 330 CPU]',
644644
' - show [thread Thread with 350 CPU]',
645-
' - show [thread Thread with 380 CPU]',
645+
' - show [thread Thread with 380 CPU] SELECTED',
646646
]);
647647
});
648648
});
@@ -1899,9 +1899,9 @@ describe('actions/receive-profile', function () {
18991899

19001900
store.dispatch(viewProfile(resultProfile));
19011901
expect(getHumanReadableTracks(store.getState())).toEqual([
1902-
'show [thread Empty default] SELECTED',
19031902
'show [thread Empty default]',
1904-
'show [thread Diff between 1 and 2 comparison]',
1903+
'show [thread Empty default]',
1904+
'show [thread Diff between 1 and 2 comparison] SELECTED',
19051905
]);
19061906
});
19071907

src/test/store/useful-tabs.test.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,12 @@ describe('getUsefulTabs', function () {
5353
it('shows only the call tree when a diffing track is selected', function () {
5454
const { profile } = getMergedProfileFromTextSamples(['A B C', 'A B B']);
5555
const { getState, dispatch } = storeWithProfile(profile);
56+
dispatch({
57+
type: 'SELECT_TRACK',
58+
selectedThreadIndexes: new Set([0]),
59+
selectedTab: 'calltree',
60+
lastNonShiftClickInformation: null,
61+
});
5662
expect(selectedThreadSelectors.getUsefulTabs(getState())).toEqual([
5763
'calltree',
5864
'flame-graph',

0 commit comments

Comments
 (0)