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
5 changes: 0 additions & 5 deletions locales/en-US/app.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -223,11 +223,6 @@ FooterLinks--hide-button =
## The timeline component of the full view in the analysis UI at the top of the
## page.

FullTimeline--graph-type = Graph type:
FullTimeline--categories-with-cpu = Categories with CPU
FullTimeline--categories = Categories
FullTimeline--stack-height = Stack height

# This string is used as the text of the track selection button.
# Displays the ratio of visible tracks count to total tracks count in the timeline.
# We have spans here to make the numbers bold.
Expand Down
28 changes: 5 additions & 23 deletions src/actions/receive-profile.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import {
getProfile,
getView,
getRelevantPagesForActiveTab,
getIsCPUUtilizationProvided,
getSymbolServerUrl,
getActiveTabID,
} from 'firefox-profiler/selectors';
Expand All @@ -58,6 +57,7 @@ import { computeActiveTabTracks } from 'firefox-profiler/profile-logic/active-ta
import { setDataSource } from './profile-view';
import { fatalError } from './errors';
import { GOOGLE_STORAGE_BUCKET } from 'firefox-profiler/app-logic/constants';
import { determineTimelineType } from 'firefox-profiler/profile-logic/profile-data';

import type {
RequestedLib,
Expand Down Expand Up @@ -335,20 +335,9 @@ export function finalizeFullProfileView(
profile
);

// Check the profile to see if we have threadCPUDelta values and switch to
// the category view with CPU if we have. This is needed only while we are
// still experimenting with the new activity graph. We should remove this
// when we have this on by default.
let timelineType = null;
if (
!hasUrlInfo &&
profile.meta.sampleUnits &&
profile.threads.some((thread) => thread.samples.threadCPUDelta)
) {
const hasCPUDeltaValues = getIsCPUUtilizationProvided(getState());
if (hasCPUDeltaValues) {
timelineType = 'cpu-category';
}
if (!hasUrlInfo) {
timelineType = determineTimelineType(profile);
}

withHistoryReplaceStateSync(() => {
Expand Down Expand Up @@ -588,15 +577,8 @@ export function finalizeActiveTabProfileView(
// still experimenting with the new activity graph. We should remove this
// when we have this on by default.
let timelineType = null;
if (
!hasUrlInfo &&
profile.meta.sampleUnits &&
profile.threads[0].samples.threadCPUDelta
) {
const hasCPUDeltaValues = getIsCPUUtilizationProvided(getState());
if (hasCPUDeltaValues) {
timelineType = 'cpu-category';
}
if (!hasUrlInfo) {
timelineType = determineTimelineType(profile);
}

dispatch({
Expand Down
2 changes: 0 additions & 2 deletions src/app-logic/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ export const TIMELINE_MARGIN_RIGHT = 15;
export const TIMELINE_MARGIN_LEFT = 150;
export const ACTIVE_TAB_TIMELINE_MARGIN_LEFT = 0;

export const TIMELINE_SETTINGS_HEIGHT = 26;

// Export the value for tests, and for computing the max height of the timeline
// for the splitter.
export const FULL_TRACK_SCREENSHOT_HEIGHT = 50;
Expand Down
31 changes: 25 additions & 6 deletions src/app-logic/url-handling.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import {
} from '../utils/uintarray-encoding';
import { tabSlugs } from '../app-logic/tabs-handling';

export const CURRENT_URL_VERSION = 6;
export const CURRENT_URL_VERSION = 7;

/**
* This static piece of state might look like an anti-pattern, but it's a relatively
Expand Down Expand Up @@ -366,10 +366,9 @@ export function getQueryStringFromUrlState(urlState: UrlState): string {
? undefined
: urlState.profileSpecific.implementation,
timelineType:
// The default is the category view, so only add it to the URL if it's the
// stack or cpu-category view.
// TODO: We should make the 'cpu-category' the new default with an upgrader.
urlState.profileSpecific.timelineType === 'category'
// The default is the cpu-category view, so only add it to the URL if it's
// the stack or category view.
urlState.profileSpecific.timelineType === 'cpu-category'
? undefined
: urlState.profileSpecific.timelineType,
Comment on lines +371 to 373

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering about not adding it at all if the type is the default (computed) one. But we'd need the profile data, and we don't have it here. Then I'm fine with this.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think it should be easiest to make the 'cpu-category' the default. Otherwise we need the profile which makes it harder.

}: BaseQueryShape);
Expand Down Expand Up @@ -1088,6 +1087,26 @@ const _upgraders: {|
delete query.transforms;
}
},
[7]: ({ query }: ProcessedLocationBeforeUpgrade) => {
// Default timeline type has been changed to 'cpu-category' from 'category'.
// Default timeline type isn't needed to be in the url, revert the values in
// the query to reflect that.
switch (query.timelineType) {
case 'cpu-category':
// This is the default value now. It's not needed in the url.
delete query.timelineType;
break;
case 'stack':
// Do nothing for this.
break;
case 'category':
default:
// We can either have 'category' or nothing for this value. We should
// explicitly add for this case.
query.timelineType = 'category';
break;
}
},
};

for (let destVersion = 1; destVersion <= CURRENT_URL_VERSION; destVersion++) {
Expand Down Expand Up @@ -1213,6 +1232,6 @@ function validateTimelineType(type: ?string): TimelineType {
default:
// Type assert we've checked everything:
(timelineType: empty);
return 'category';
return 'cpu-category';
}
}
9 changes: 8 additions & 1 deletion src/components/shared/thread/ActivityGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import type {
SelectedState,
Milliseconds,
CssPixels,
TimelineType,
} from 'firefox-profiler/types';
import type {
ActivityFillGraphQuerier,
Expand Down Expand Up @@ -54,6 +55,7 @@ export type Props = {|
+enableCPUUsage: boolean,
+maxThreadCPUDeltaPerMs: number,
+implementationFilter: ImplementationFilter,
+timelineType: TimelineType,
...SizeProps,
|};

Expand Down Expand Up @@ -169,6 +171,7 @@ class ThreadActivityGraphImpl extends React.PureComponent<Props, State> {
implementationFilter,
width,
height,
timelineType,
} = this.props;
const { hoveredPixelState, mouseX, mouseY } = this.state;
return (
Expand Down Expand Up @@ -204,7 +207,11 @@ class ThreadActivityGraphImpl extends React.PureComponent<Props, State> {
<Tooltip mouseX={mouseX} mouseY={mouseY}>
<SampleTooltipContents
sampleIndex={hoveredPixelState.sample}
cpuRatioInTimeRange={hoveredPixelState.cpuRatioInTimeRange}
cpuRatioInTimeRange={
timelineType === 'cpu-category'
? hoveredPixelState.cpuRatioInTimeRange
: null
}
rangeFilteredThread={rangeFilteredThread}
categories={categories}
implementationFilter={implementationFilter}
Expand Down
Loading