Skip to content

Commit

Permalink
Reduce confusion between call tree summary strategy aware samples and…
Browse files Browse the repository at this point in the history
… regular samples (#5330)

Fixes #5326, but leaves #5327 unfixed for now.
  • Loading branch information
mstange authored Jan 17, 2025
2 parents a28d057 + 1edbe67 commit 5431546
Show file tree
Hide file tree
Showing 6 changed files with 118 additions and 95 deletions.
18 changes: 9 additions & 9 deletions src/components/flame-graph/Canvas.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export type OwnProps = {|
+weightType: WeightType,
+innerWindowIDToPageMap: Map<InnerWindowID, Page> | null,
+unfilteredThread: Thread,
+sampleIndexOffset: number,
+ctssSampleIndexOffset: number,
+maxStackDepthPlusOne: number,
+flameGraphTiming: FlameGraphTiming,
+callNodeInfo: CallNodeInfo,
Expand All @@ -75,8 +75,8 @@ export type OwnProps = {|
+interval: Milliseconds,
+isInverted: boolean,
+callTreeSummaryStrategy: CallTreeSummaryStrategy,
+samples: SamplesLikeTable,
+unfilteredSamples: SamplesLikeTable,
+ctssSamples: SamplesLikeTable,
+unfilteredCtssSamples: SamplesLikeTable,
+tracedTiming: CallTreeTimings | null,
+displayImplementation: boolean,
+displayStackType: boolean,
Expand Down Expand Up @@ -360,7 +360,7 @@ class FlameGraphCanvasImpl extends React.PureComponent<Props> {
const {
thread,
unfilteredThread,
sampleIndexOffset,
ctssSampleIndexOffset,
flameGraphTiming,
callTree,
callNodeInfo,
Expand All @@ -370,8 +370,8 @@ class FlameGraphCanvasImpl extends React.PureComponent<Props> {
callTreeSummaryStrategy,
innerWindowIDToPageMap,
weightType,
samples,
unfilteredSamples,
ctssSamples,
unfilteredCtssSamples,
tracedTiming,
displayImplementation,
displayStackType,
Expand Down Expand Up @@ -433,10 +433,10 @@ class FlameGraphCanvasImpl extends React.PureComponent<Props> {
callNodeInfo,
interval,
unfilteredThread,
sampleIndexOffset,
ctssSampleIndexOffset,
categories,
samples,
unfilteredSamples,
ctssSamples,
unfilteredCtssSamples,
displayImplementation
)
: undefined
Expand Down
29 changes: 14 additions & 15 deletions src/components/flame-graph/FlameGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ type StateProps = {|
+weightType: WeightType,
+innerWindowIDToPageMap: Map<InnerWindowID, Page> | null,
+unfilteredThread: Thread,
+sampleIndexOffset: number,
+ctssSampleIndexOffset: number,
+maxStackDepthPlusOne: number,
+timeRange: StartEndRange,
+previewSelection: PreviewSelection,
Expand All @@ -87,8 +87,8 @@ type StateProps = {|
+interval: Milliseconds,
+isInverted: boolean,
+callTreeSummaryStrategy: CallTreeSummaryStrategy,
+samples: SamplesLikeTable,
+unfilteredSamples: SamplesLikeTable,
+ctssSamples: SamplesLikeTable,
+unfilteredCtssSamples: SamplesLikeTable,
+tracedTiming: CallTreeTimings | null,
+displayImplementation: boolean,
+displayStackType: boolean,
Expand Down Expand Up @@ -320,7 +320,7 @@ class FlameGraphImpl extends React.PureComponent<Props> {
const {
thread,
unfilteredThread,
sampleIndexOffset,
ctssSampleIndexOffset,
threadsKey,
maxStackDepthPlusOne,
flameGraphTiming,
Expand All @@ -337,8 +337,8 @@ class FlameGraphImpl extends React.PureComponent<Props> {
isInverted,
innerWindowIDToPageMap,
weightType,
samples,
unfilteredSamples,
ctssSamples,
unfilteredCtssSamples,
tracedTiming,
displayImplementation,
displayStackType,
Expand Down Expand Up @@ -375,7 +375,7 @@ class FlameGraphImpl extends React.PureComponent<Props> {
innerWindowIDToPageMap,
weightType,
unfilteredThread,
sampleIndexOffset,
ctssSampleIndexOffset,
maxStackDepthPlusOne,
flameGraphTiming,
callTree,
Expand All @@ -392,8 +392,8 @@ class FlameGraphImpl extends React.PureComponent<Props> {
shouldDisplayTooltips: this._shouldDisplayTooltips,
interval,
isInverted,
samples,
unfilteredSamples,
ctssSamples,
unfilteredCtssSamples,
tracedTiming,
displayImplementation,
displayStackType,
Expand All @@ -418,8 +418,6 @@ export const FlameGraph = explicitConnect<{||}, StateProps, DispatchProps>({
thread: selectedThreadSelectors.getFilteredThread(state),
unfilteredThread: selectedThreadSelectors.getThread(state),
weightType: selectedThreadSelectors.getWeightTypeForCallTree(state),
sampleIndexOffset:
selectedThreadSelectors.getSampleIndexOffsetFromPreviewRange(state),
// Use the filtered call node max depth, rather than the preview filtered one, so
// that the viewport height is stable across preview selections.
maxStackDepthPlusOne:
Expand All @@ -441,10 +439,11 @@ export const FlameGraph = explicitConnect<{||}, StateProps, DispatchProps>({
callTreeSummaryStrategy:
selectedThreadSelectors.getCallTreeSummaryStrategy(state),
innerWindowIDToPageMap: getInnerWindowIDToPageMap(state),
samples:
selectedThreadSelectors.getPreviewFilteredSamplesForCallTree(state),
unfilteredSamples:
selectedThreadSelectors.getUnfilteredSamplesForCallTree(state),
ctssSamples: selectedThreadSelectors.getPreviewFilteredCtssSamples(state),
ctssSampleIndexOffset:
selectedThreadSelectors.getPreviewFilteredCtssSampleIndexOffset(state),
unfilteredCtssSamples:
selectedThreadSelectors.getUnfilteredCtssSamples(state),
tracedTiming: selectedThreadSelectors.getTracedTiming(state),
displayImplementation: getProfileUsesFrameImplementation(state),
displayStackType: getProfileUsesMultipleStackTypes(state),
Expand Down
3 changes: 1 addition & 2 deletions src/components/timeline/TrackThread.js
Original file line number Diff line number Diff line change
Expand Up @@ -359,8 +359,7 @@ export const TimelineTrackThread = explicitConnect<
interval: getProfileInterval(state),
rangeStart: committedRange.start,
rangeEnd: committedRange.end,
sampleIndexOffset:
selectors.getSampleIndexOffsetFromCommittedRange(state),
sampleIndexOffset: selectors.getFilteredSampleIndexOffset(state),
categories: getCategories(state),
timelineType,
hasFileIoMarkers:
Expand Down
10 changes: 5 additions & 5 deletions src/selectors/per-thread/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -264,10 +264,10 @@ export const selectedNodeSelectors: NodeSelectors = (() => {
selectedThreadSelectors.getCallNodeInfo,
ProfileSelectors.getProfileInterval,
selectedThreadSelectors.getThread,
selectedThreadSelectors.getSampleIndexOffsetFromPreviewRange,
selectedThreadSelectors.getPreviewFilteredCtssSampleIndexOffset,
ProfileSelectors.getCategories,
selectedThreadSelectors.getPreviewFilteredSamplesForCallTree,
selectedThreadSelectors.getUnfilteredSamplesForCallTree,
selectedThreadSelectors.getPreviewFilteredCtssSamples,
selectedThreadSelectors.getUnfilteredCtssSamples,
ProfileSelectors.getProfileUsesFrameImplementation,
ProfileData.getTimingsForPath
);
Expand Down Expand Up @@ -307,7 +307,7 @@ export const selectedNodeSelectors: NodeSelectors = (() => {

const getSourceViewLineTimings: Selector<LineTimings> = createSelector(
getSourceViewStackLineInfo,
selectedThreadSelectors.getPreviewFilteredSamplesForCallTree,
selectedThreadSelectors.getPreviewFilteredCtssSamples,
getLineTimings
);

Expand Down Expand Up @@ -339,7 +339,7 @@ export const selectedNodeSelectors: NodeSelectors = (() => {
const getAssemblyViewAddressTimings: Selector<AddressTimings> =
createSelector(
getAssemblyViewStackAddressInfo,
selectedThreadSelectors.getPreviewFilteredSamplesForCallTree,
selectedThreadSelectors.getPreviewFilteredCtssSamples,
getAddressTimings
);

Expand Down
27 changes: 13 additions & 14 deletions src/selectors/per-thread/stack-sample.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,11 +242,10 @@ export function getStackAndSampleSelectorsPerThread(
)
);

const getSampleIndexToCallNodeIndexForPreviewFilteredThread: Selector<
const _getPreviewFilteredCtssSampleIndexToCallNodeIndex: Selector<
Array<IndexIntoCallNodeTable | null>,
> = createSelector(
(state) =>
threadSelectors.getPreviewFilteredSamplesForCallTree(state).stack,
(state) => threadSelectors.getPreviewFilteredCtssSamples(state).stack,
(state) => getCallNodeInfo(state).getStackIndexToCallNodeIndex(),
ProfileData.getSampleIndexToCallNodeIndex
);
Expand Down Expand Up @@ -305,14 +304,14 @@ export function getStackAndSampleSelectorsPerThread(
);

const getFilteredCallNodeMaxDepthPlusOne: Selector<number> = createSelector(
threadSelectors.getFilteredSamplesForCallTree,
threadSelectors.getFilteredCtssSamples,
getCallNodeInfo,
ProfileData.computeCallNodeMaxDepthPlusOne
);

const getPreviewFilteredCallNodeMaxDepthPlusOne: Selector<number> =
createSelector(
threadSelectors.getPreviewFilteredSamplesForCallTree,
threadSelectors.getPreviewFilteredCtssSamples,
getCallNodeInfo,
ProfileData.computeCallNodeMaxDepthPlusOne
);
Expand All @@ -325,13 +324,13 @@ export function getStackAndSampleSelectorsPerThread(
*/
const getWeightTypeForCallTree: Selector<WeightType> = createSelector(
// The weight type is not changed by the filtering done on the profile.
threadSelectors.getUnfilteredSamplesForCallTree,
threadSelectors.getUnfilteredCtssSamples,
(samples) => samples.weightType || 'samples'
);

const getCallTreeTimings: Selector<CallTree.CallTreeTimings> = createSelector(
threadSelectors.getPreviewFilteredSamplesForCallTree,
getSampleIndexToCallNodeIndexForPreviewFilteredThread,
threadSelectors.getPreviewFilteredCtssSamples,
_getPreviewFilteredCtssSampleIndexToCallNodeIndex,
getCallNodeInfo,
(samples, sampleIndexToCallNodeIndex, callNodeInfo) => {
const callNodeLeafAndSummary = CallTree.computeCallNodeLeafAndSummary(
Expand All @@ -357,21 +356,21 @@ export function getStackAndSampleSelectorsPerThread(

const getSourceViewLineTimings: Selector<LineTimings> = createSelector(
getSourceViewStackLineInfo,
threadSelectors.getPreviewFilteredSamplesForCallTree,
threadSelectors.getPreviewFilteredCtssSamples,
getLineTimings
);

const getAssemblyViewAddressTimings: Selector<AddressTimings> =
createSelector(
getAssemblyViewStackAddressInfo,
threadSelectors.getPreviewFilteredSamplesForCallTree,
threadSelectors.getPreviewFilteredCtssSamples,
getAddressTimings
);

const getTracedTiming: Selector<CallTree.CallTreeTimings | null> =
createSelector(
threadSelectors.getPreviewFilteredSamplesForCallTree,
getSampleIndexToCallNodeIndexForPreviewFilteredThread,
threadSelectors.getPreviewFilteredCtssSamples,
_getPreviewFilteredCtssSampleIndexToCallNodeIndex,
getCallNodeInfo,
ProfileSelectors.getProfileInterval,
(samples, sampleIndexToCallNodeIndex, callNodeInfo, interval) => {
Expand Down Expand Up @@ -408,8 +407,8 @@ export function getStackAndSampleSelectorsPerThread(

const getStackTimingByDepth: Selector<StackTiming.StackTimingByDepth> =
createSelector(
threadSelectors.getFilteredSamplesForCallTree,
getSampleIndexToCallNodeIndexForFilteredThread,
threadSelectors.getFilteredCtssSamples,
getSampleIndexToCallNodeIndexForFilteredThread, // Bug! #5327
getCallNodeInfo,
getFilteredCallNodeMaxDepthPlusOne,
ProfileSelectors.getProfileInterval,
Expand Down
Loading

0 comments on commit 5431546

Please sign in to comment.