Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 37 additions & 5 deletions src/profile-logic/tracks.js
Original file line number Diff line number Diff line change
Expand Up @@ -711,11 +711,7 @@ function _computeHiddenTracksForVisibleThreads(
const hiddenLocalTracks = new Set(
localTrackOrder.filter((localTrackIndex) => {
const localTrack = localTracks[localTrackIndex];
if (localTrack.type !== 'thread') {
// Keep non-thread local tracks visible.
return false;
}
return !visibleThreadIndexes.has(localTrack.threadIndex);
return !_isLocalTrackVisible(localTrack, visibleThreadIndexes);
})
);
hiddenLocalTracksByPid.set(pid, hiddenLocalTracks);
Expand Down Expand Up @@ -1340,3 +1336,39 @@ export function getTrackReferenceFromThreadIndex(
// Failed to find the thread from its thread index.
return null;
}

/*
* Returns whether the local track should be visible or not.
* If the track is not a thread, some of them can be visible by default and some
* of them can be hidden to reduce the noise. This mostly depends on either the
* usefulness or the activity of that track.
*
* TODO: Check the memory track activity here to decide if it should be visible.
*/
function _isLocalTrackVisible(
localTrack: LocalTrack,
visibleThreadIndexes: Set<ThreadIndex>
): boolean {
switch (localTrack.type) {
case 'thread':
// Show the local thread if it's included in the visible thread indexes.
return visibleThreadIndexes.has(localTrack.threadIndex);
case 'network':
case 'memory':
// 'event-delay' and 'process-cpu' tracks are experimental and they should
// be visible by default whenever they are included in a profile. (fallthrough)
case 'event-delay':
case 'process-cpu':
// Power tracks are there only if the power feature is enabled. So they should
// be visible by default whenever they're included in a profile. (fallthrough)
case 'power':
// Keep non-thread local tracks visible.
return true;
case 'ipc':
// IPC tracks are not always useful to the users. So we are making them hidden
// by default to reduce the noise.
return false;
default:
throw assertExhaustiveCheck(localTrack, 'Unhandled LocalTrack type.');
}
}
30 changes: 28 additions & 2 deletions src/test/components/LocalTrack.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { render } from 'firefox-profiler/test/fixtures/testing-library';
import {
changeSelectedThreads,
hideLocalTrack,
showLocalTrack,
} from '../../actions/profile-view';
import { TimelineLocalTrack } from '../../components/timeline/LocalTrack';
import {
Expand Down Expand Up @@ -130,13 +131,37 @@ describe('timeline/LocalTrack', function () {
});

describe('with an IPC track', function () {
it('appears hidden by default', function () {
const { container } = setupWithIPC();
expect(container.querySelector('.timelineTrackHidden')).toBeTruthy();
expect(container.querySelector('.timelineTrack')).toBeFalsy();
});

it('can be shown', function () {
const { dispatch, pid, trackReference, container } = setupWithIPC();

// First check that the IPC track is hidden by default.
expect(container.querySelector('.timelineTrackHidden')).toBeTruthy();
expect(container.querySelector('.timelineTrack')).toBeFalsy();

// Now make it visible and check it.
dispatch(showLocalTrack(pid, trackReference.trackIndex));
expect(container.querySelector('.timelineTrackHidden')).toBeFalsy();
expect(container.querySelector('.timelineTrack')).toBeTruthy();
});

it('correctly renders the IPC label', function () {
const { getLocalTrackLabel } = setupWithIPC();
const { dispatch, pid, trackReference, getLocalTrackLabel } =
setupWithIPC();
dispatch(showLocalTrack(pid, trackReference.trackIndex));
expect(getLocalTrackLabel()).toHaveTextContent('IPC — Empty');
});

it('matches the snapshot of the IPC track', () => {
const { container } = setupWithIPC();
const { pid, dispatch, trackReference, container, flushRafCalls } =
setupWithIPC();
dispatch(showLocalTrack(pid, trackReference.trackIndex));
flushRafCalls();
expect(container.firstChild).toMatchSnapshot();
});
});
Expand Down Expand Up @@ -191,6 +216,7 @@ function setup(
pid: PID,
getLocalTrackLabel,
getLocalTrackRow,
flushRafCalls,
};
}

Expand Down
32 changes: 16 additions & 16 deletions src/test/components/MarkerTable.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -334,13 +334,13 @@ describe('MarkerTable', function () {
// Make sure that it's hidden.
expect(getHumanReadableTracks(getState())).toEqual([
'hide [thread GeckoMain default]',
' - show [ipc GeckoMain]',
' - hide [ipc GeckoMain]',
'show [thread GeckoMain tab] SELECTED',
' - show [ipc GeckoMain] SELECTED',
' - hide [ipc GeckoMain] SELECTED',
' - show [thread DOM Worker]',
' - show [ipc DOM Worker]',
' - hide [ipc DOM Worker]',
' - show [thread Style]',
' - show [ipc Style]',
' - hide [ipc Style]',
]);

// Check the actual behavior now.
Expand All @@ -352,13 +352,13 @@ describe('MarkerTable', function () {
// Make sure that it's not hidden anymore.
expect(getHumanReadableTracks(getState())).toEqual([
'show [thread GeckoMain default] SELECTED',
' - show [ipc GeckoMain] SELECTED',
' - hide [ipc GeckoMain] SELECTED',
'show [thread GeckoMain tab]',
' - show [ipc GeckoMain]',
' - hide [ipc GeckoMain]',
' - show [thread DOM Worker]',
' - show [ipc DOM Worker]',
' - hide [ipc DOM Worker]',
' - show [thread Style]',
' - show [ipc Style]',
' - hide [ipc Style]',
]);
});

Expand Down Expand Up @@ -392,13 +392,13 @@ describe('MarkerTable', function () {
// Make sure that they are hidden.
expect(getHumanReadableTracks(getState())).toEqual([
'show [thread GeckoMain default] SELECTED',
' - show [ipc GeckoMain] SELECTED',
' - hide [ipc GeckoMain] SELECTED',
'hide [thread GeckoMain tab]',
' - show [ipc GeckoMain]',
' - hide [ipc GeckoMain]',
' - hide [thread DOM Worker]',
' - show [ipc DOM Worker]',
' - hide [ipc DOM Worker]',
' - show [thread Style]',
' - show [ipc Style]',
' - hide [ipc Style]',
]);

// Check the actual behavior now.
Expand All @@ -410,13 +410,13 @@ describe('MarkerTable', function () {
// Make sure that they are not hidden anymore.
expect(getHumanReadableTracks(getState())).toEqual([
'show [thread GeckoMain default]',
' - show [ipc GeckoMain]',
' - hide [ipc GeckoMain]',
'show [thread GeckoMain tab]',
' - show [ipc GeckoMain]',
' - hide [ipc GeckoMain]',
' - show [thread DOM Worker] SELECTED',
' - show [ipc DOM Worker] SELECTED',
' - hide [ipc DOM Worker] SELECTED',
' - show [thread Style]',
' - show [ipc Style]',
' - hide [ipc Style]',
]);
});

Expand Down
26 changes: 23 additions & 3 deletions src/test/components/Timeline.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ import {
selectedThreadSelectors,
getRightClickedTrack,
getMouseTimePosition,
getLocalTracksByPid,
} from 'firefox-profiler/selectors';
import { FULL_TRACK_SCREENSHOT_HEIGHT } from 'firefox-profiler/app-logic/constants';
import { ensureExists } from 'firefox-profiler/utils/flow';
import { showLocalTrack } from 'firefox-profiler/actions/profile-view';

import { storeWithProfile } from '../fixtures/stores';
import {
Expand Down Expand Up @@ -79,7 +81,23 @@ describe('Timeline multiple thread selection', function () {
);
flushRafCalls();

return { ...renderResult, ...store };
const showAllIPCTracks = () => {
const localTracksByPid = getLocalTracksByPid(store.getState());
for (const [pid, localTracks] of localTracksByPid) {
for (
let trackIndex = 0;
trackIndex < localTracks.length;
trackIndex++
) {
const localTrack = localTracks[trackIndex];
if (localTrack.type === 'ipc') {
store.dispatch(showLocalTrack(pid, trackIndex));
}
}
}
};

return { ...renderResult, ...store, showAllIPCTracks };
}

it('can toggle select multiple threads', function () {
Expand Down Expand Up @@ -994,7 +1012,8 @@ describe('Timeline multiple thread selection', function () {
profile.threads[7] // DOM Worker
);

const { getState } = setup(profile);
const { getState, showAllIPCTracks } = setup(profile);
showAllIPCTracks();
expect(getHumanReadableTracks(getState())).toEqual([
'show [thread GeckoMain default]',
' - show [ipc GeckoMain]',
Expand Down Expand Up @@ -1092,7 +1111,8 @@ describe('Timeline multiple thread selection', function () {
profile.threads[7] // DOM Worker
);

const { getState } = setup(profile);
const { getState, showAllIPCTracks } = setup(profile);
showAllIPCTracks();
expect(getHumanReadableTracks(getState())).toEqual([
'show [thread GeckoMain default]',
' - show [ipc GeckoMain]',
Expand Down
4 changes: 2 additions & 2 deletions src/test/store/tracks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -671,10 +671,10 @@ describe('ordering and hiding', function () {
'show [thread GeckoMain default]',
'show [thread GeckoMain tab] SELECTED',
// Belongs to the global tab track.
' - show [ipc GeckoMain] SELECTED',
' - hide [ipc GeckoMain] SELECTED',
' - show [thread DOM Worker]',
// Belongs to the DOM Worker local track.
' - show [ipc DOM Worker]',
' - hide [ipc DOM Worker]',
' - show [thread Style]',
]);
});
Expand Down