Skip to content

Commit

Permalink
Add a mechanism to change the summary strategy and store it in the UR…
Browse files Browse the repository at this point in the history
  • Loading branch information
gregtatum authored Jul 17, 2019
2 parents 4a1e0bb + 134eaac commit fe7443a
Show file tree
Hide file tree
Showing 9 changed files with 129 additions and 1 deletion.
22 changes: 22 additions & 0 deletions src/actions/profile-view.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import { objectShallowEquals } from '../utils/index';
import type {
PreviewSelection,
ImplementationFilter,
CallTreeSummaryStrategy,
TrackReference,
TimelineType,
DataSource,
Expand Down Expand Up @@ -939,6 +940,27 @@ export function changeImplementationFilter(
};
}

/**
* This action changes the strategy used to build and display the call tree. This could
* use sample data, or build a new call tree based off of allocation information stored
* in markers.
*/
export function changeCallTreeSummaryStrategy(
strategy: CallTreeSummaryStrategy
): Action {
sendAnalytics({
hitType: 'event',
eventCategory: 'profile',
eventAction: 'change call tree summary strategy',
eventLabel: strategy,
});

return {
type: 'CHANGE_CALL_TREE_SUMMARY_STRATEGY',
strategy,
};
}

export function changeInvertCallstack(
invertCallstack: boolean
): ThunkAction<void> {
Expand Down
8 changes: 8 additions & 0 deletions src/app-logic/url-handling.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
toValidTabSlug,
ensureExists,
} from '../utils/flow';
import { toValidCallTreeSummaryStrategy } from '../profile-logic/profile-data';
import { oneLine } from 'common-tags';
import type { UrlState } from '../types/state';
import type { DataSource } from '../types/actions';
Expand Down Expand Up @@ -105,6 +106,7 @@ type CallTreeQuery = {|
search: string, // "js::RunScript"
invertCallstack: null | void,
implementation: string,
ctSummary: string,
|};

type MarkersQuery = {|
Expand All @@ -122,6 +124,7 @@ type StackChartQuery = {|
search: string, // "js::RunScript"
invertCallstack: null | void,
implementation: string,
ctSummary: string,
|};

type JsTracerQuery = {|
Expand Down Expand Up @@ -242,6 +245,10 @@ export function urlStateToUrlObject(urlState: UrlState): UrlObject {
urlState.profileSpecific.transforms[selectedThread]
) || undefined;
}
query.ctSummary =
urlState.profileSpecific.callTreeSummaryStrategy === 'timing'
? undefined
: urlState.profileSpecific.callTreeSummaryStrategy;
break;
}
case 'marker-table':
Expand Down Expand Up @@ -359,6 +366,7 @@ export function stateFromLocation(
profileName: query.profileName,
profileSpecific: {
implementation,
callTreeSummaryStrategy: toValidCallTreeSummaryStrategy(query.ctSummary),
invertCallstack: query.invertCallstack !== undefined,
showJsTracerSummary: query.summary !== undefined,
committedRanges: query.range ? parseCommittedRanges(query.range) : [],
Expand Down
23 changes: 22 additions & 1 deletion src/profile-logic/profile-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ import { assertExhaustiveCheck } from '../utils/flow';
import type { Milliseconds, StartEndRange } from '../types/units';
import { timeCode } from '../utils/time-code';
import { hashPath } from '../utils/path';
import type { ImplementationFilter } from '../types/actions';
import type {
ImplementationFilter,
CallTreeSummaryStrategy,
} from '../types/actions';
import bisection from 'bisection';
import type { UniqueStringArray } from '../utils/unique-string-array';

Expand Down Expand Up @@ -685,6 +688,24 @@ export function toValidImplementationFilter(
}
}

export function toValidCallTreeSummaryStrategy(
strategy: mixed
): CallTreeSummaryStrategy {
switch (strategy) {
case 'timing':
case 'js-allocations':
case 'native-allocations':
return strategy;
default:
// Default to "timing" if the strategy is not recognized. This value can come
// from a user-generated URL.
// e.g. `profiler.firefox.com/public/hash/ctSummary=tiiming` (note the typo.)
// This default branch will ensure we don't send values we don't understand to
// the store.
return 'timing';
}
}

export function filterThreadByImplementation(
thread: Thread,
implementation: string,
Expand Down
18 changes: 18 additions & 0 deletions src/reducers/url-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type { TransformStacksPerThread } from '../types/transforms';
import type {
DataSource,
ImplementationFilter,
CallTreeSummaryStrategy,
TimelineType,
} from '../types/actions';
import type { UrlState, Reducer } from '../types/state';
Expand Down Expand Up @@ -228,6 +229,22 @@ const implementation: Reducer<ImplementationFilter> = (
}
};

/**
* Represents the current strategy used to summarize numeric information in the call
* call tree.
*/
const callTreeSummaryStrategy: Reducer<CallTreeSummaryStrategy> = (
state = 'timing',
action
) => {
switch (action.type) {
case 'CHANGE_CALL_TREE_SUMMARY_STRATEGY':
return action.strategy;
default:
return state;
}
};

const invertCallstack: Reducer<boolean> = (state = false, action) => {
switch (action.type) {
case 'CHANGE_INVERT_CALLSTACK':
Expand Down Expand Up @@ -381,6 +398,7 @@ const profileSpecific = combineReducers({
hiddenLocalTracksByPid,
localTrackOrderByPid,
implementation,
callTreeSummaryStrategy,
invertCallstack,
showJsTracerSummary,
committedRanges,
Expand Down
3 changes: 3 additions & 0 deletions src/selectors/url-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type {
TimelineType,
DataSource,
ImplementationFilter,
CallTreeSummaryStrategy,
} from '../types/actions';
import type { TabSlug } from '../app-logic/tabs-handling';
import type { UrlState } from '../types/state';
Expand Down Expand Up @@ -45,6 +46,8 @@ export const getAllCommittedRanges: Selector<StartEndRange[]> = state =>
getProfileSpecificState(state).committedRanges;
export const getImplementationFilter: Selector<ImplementationFilter> = state =>
getProfileSpecificState(state).implementation;
export const getCallTreeSummaryStrategy: Selector<CallTreeSummaryStrategy> = state =>
getProfileSpecificState(state).callTreeSummaryStrategy;
export const getInvertCallstack: Selector<boolean> = state =>
getProfileSpecificState(state).invertCallstack;
export const getShowJsTracerSummary: Selector<boolean> = state =>
Expand Down
13 changes: 13 additions & 0 deletions src/test/store/profile-view.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1544,3 +1544,16 @@ describe('counter selectors', function() {
});
});
});

describe('call tree summary strategy', function() {
it('can change the call tree strategy', function() {
const { dispatch, getState } = storeWithProfile();
expect(UrlStateSelectors.getCallTreeSummaryStrategy(getState())).toEqual(
'timing'
);
dispatch(ProfileView.changeCallTreeSummaryStrategy('js-allocations'));
expect(UrlStateSelectors.getCallTreeSummaryStrategy(getState())).toEqual(
'js-allocations'
);
});
});
32 changes: 32 additions & 0 deletions src/test/url-handling.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -843,3 +843,35 @@ describe('compare', function() {
expect(resultingUrl).toMatch(`profiles[]=${encodeURIComponent(url2)}`);
});
});

describe('call tree summary strategy', function() {
const { getCallTreeSummaryStrategy } = urlStateReducers;

it('defaults to timing', function() {
const { getState } = _getStoreWithURL();
expect(getCallTreeSummaryStrategy(getState())).toEqual('timing');
});

it('can be js allocations', function() {
const { getState } = _getStoreWithURL({
search: '?ctSummary=js-allocations',
});
expect(getCallTreeSummaryStrategy(getState())).toEqual('js-allocations');
});

it('can be native allocations', function() {
const { getState } = _getStoreWithURL({
search: '?ctSummary=native-allocations',
});
expect(getCallTreeSummaryStrategy(getState())).toEqual(
'native-allocations'
);
});

it('will use the default "timing" when an unknown value is received', function() {
const { getState } = _getStoreWithURL({
search: '?ctSummary=unknown-value',
});
expect(getCallTreeSummaryStrategy(getState())).toEqual('timing');
});
});
9 changes: 9 additions & 0 deletions src/types/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ export type RequestedLib = {|
+breakpadId: string,
|};
export type ImplementationFilter = 'combined' | 'js' | 'cpp';
// Change the strategy for computing the summarizing information for the call tree.
export type CallTreeSummaryStrategy =
| 'timing'
| 'js-allocations'
| 'native-allocations';

/**
* This type determines what kind of information gets sanitized from published profiles.
Expand Down Expand Up @@ -306,6 +311,10 @@ type UrlStateAction =
+previousImplementation: ImplementationFilter,
+implementation: ImplementationFilter,
|}
| {|
type: 'CHANGE_CALL_TREE_SUMMARY_STRATEGY',
strategy: CallTreeSummaryStrategy,
|}
| {|
+type: 'CHANGE_INVERT_CALLSTACK',
+invertCallstack: boolean,
Expand Down
2 changes: 2 additions & 0 deletions src/types/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type {
DataSource,
PreviewSelection,
ImplementationFilter,
CallTreeSummaryStrategy,
RequestedLib,
TrackReference,
TimelineType,
Expand Down Expand Up @@ -173,6 +174,7 @@ export type UrlState = {|
hiddenLocalTracksByPid: Map<Pid, Set<TrackIndex>>,
localTrackOrderByPid: Map<Pid, TrackIndex[]>,
implementation: ImplementationFilter,
callTreeSummaryStrategy: CallTreeSummaryStrategy,
invertCallstack: boolean,
showJsTracerSummary: boolean,
committedRanges: StartEndRange[],
Expand Down

0 comments on commit fe7443a

Please sign in to comment.