From 71bdfefe05c24f985316e16ecce1278f6ec403cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Mon, 8 May 2023 15:35:58 +0200 Subject: [PATCH 01/10] Split the thread per-thread selectors to be able to get markers for transforms Before the 'filter-samples' transform, we didn't need to get the derived markers to be able to compute any thread related pre-thread selectors. But with 'filter-samples' transform, we need to get the derived markers to be able to compute the transform filtered thread because this transform needs to find the markers that match the filter and get the interval range out of them. So because of this, I had to split the thread related selectors so we can get the marker selectors in between them and have the marker selectors in the second phase of thread selectors. Previously the dependency graph was like this: ``` thread selectors | v stack and sample + marker selectors | v composed selectors ``` Now it's like this: ``` basic thread selectors | v marker selectors | v rest of the thread selectors | v stack and sample selectors | v composed selectors ``` We can't put these thread selectors that require markers into the `composed` selectors because stack and sample selectors require those selectors earlier in the pipeline and I wanted to keep the thread selectors still in their file to keep the pipeline in a single file. --- src/selectors/per-thread/index.js | 30 ++- src/selectors/per-thread/markers.js | 5 +- src/selectors/per-thread/stack-sample.js | 8 +- src/selectors/per-thread/thread.js | 302 ++++++++++++----------- 4 files changed, 193 insertions(+), 152 deletions(-) diff --git a/src/selectors/per-thread/index.js b/src/selectors/per-thread/index.js index 4881cc481c..5554e0f494 100644 --- a/src/selectors/per-thread/index.js +++ b/src/selectors/per-thread/index.js @@ -7,7 +7,8 @@ import memoize from 'memoize-immutable'; import * as UrlState from '../url-state'; import * as ProfileData from '../../profile-logic/profile-data'; import { - getThreadSelectorsPerThread, + getThreadSelectorsWithMarkersPerThread, + getBasicThreadSelectorsPerThread, type ThreadSelectorsPerThread, } from './thread'; import { @@ -152,12 +153,26 @@ function _buildThreadSelectors( threadIndexes: Set, threadsKey: ThreadsKey = ProfileData.getThreadsKey(threadIndexes) ) { - // We define the thread selectors in 3 steps to ensure clarity in the + // We define the thread selectors in 5 steps to ensure clarity in the // separate files. - // 1. The basic selectors. - let selectors = getThreadSelectorsPerThread(threadIndexes, threadsKey); - // 2. Stack, sample and marker selectors that need the previous basic - // selectors for their own definition. + // 1. The basic thread selectors. + let selectors = getBasicThreadSelectorsPerThread(threadIndexes, threadsKey); + // 2. The marker selectors. + selectors = { + ...selectors, + ...getMarkerSelectorsPerThread(selectors, threadIndexes, threadsKey), + }; + // 3. The thread selectors that need marker selectors. + selectors = { + ...selectors, + ...getThreadSelectorsWithMarkersPerThread( + selectors, + threadIndexes, + threadsKey + ), + }; + // 4. Stack, sample selectors that need the previous selectors for their + // own definition. selectors = { ...selectors, ...getStackAndSampleSelectorsPerThread( @@ -165,9 +180,8 @@ function _buildThreadSelectors( threadIndexes, threadsKey ), - ...getMarkerSelectorsPerThread(selectors, threadIndexes, threadsKey), }; - // 3. Other selectors that need selectors from different files to be defined. + // 5. Other selectors that need selectors from different files to be defined. selectors = { ...selectors, ...getComposedSelectorsPerThread(selectors), diff --git a/src/selectors/per-thread/markers.js b/src/selectors/per-thread/markers.js index 96b5760e20..3b0228b6e9 100644 --- a/src/selectors/per-thread/markers.js +++ b/src/selectors/per-thread/markers.js @@ -13,7 +13,7 @@ import * as ProfileSelectors from '../profile'; import { getRightClickedMarkerInfo } from '../right-clicked-marker'; import { getLabelGetter } from '../../profile-logic/marker-schema'; -import type { ThreadSelectorsPerThread } from './thread'; +import type { BasicThreadSelectorsPerThread } from './thread'; import type { RawMarkerTable, ThreadIndex, @@ -43,7 +43,7 @@ export type MarkerSelectorsPerThread = $ReturnType< * Create the selectors for a thread that have to do with either markers. */ export function getMarkerSelectorsPerThread( - threadSelectors: ThreadSelectorsPerThread, + threadSelectors: BasicThreadSelectorsPerThread, threadIndexes: Set, threadsKey: ThreadsKey ) { @@ -610,6 +610,7 @@ export function getMarkerSelectorsPerThread( getTimelineJankMarkerIndexes, getDerivedMarkerInfo, getMarkerIndexToRawMarkerIndexes, + getFullMarkerList, getFullMarkerListIndexes, getNetworkMarkerIndexes, getSearchFilteredNetworkMarkerIndexes, diff --git a/src/selectors/per-thread/stack-sample.js b/src/selectors/per-thread/stack-sample.js index 41554ee576..674eefd6cd 100644 --- a/src/selectors/per-thread/stack-sample.js +++ b/src/selectors/per-thread/stack-sample.js @@ -44,6 +44,7 @@ import type { } from 'firefox-profiler/types'; import type { ThreadSelectorsPerThread } from './thread'; +import type { MarkerSelectorsPerThread } from './markers'; /** * Infer the return type from the getStackAndSampleSelectorsPerThread function. This @@ -54,11 +55,16 @@ export type StackAndSampleSelectorsPerThread = $ReturnType< typeof getStackAndSampleSelectorsPerThread >; +type ThreadAndMarkerSelectorsPerThread = {| + ...ThreadSelectorsPerThread, + ...MarkerSelectorsPerThread, +|}; + /** * Create the selectors for a thread that have to do with either stacks or samples. */ export function getStackAndSampleSelectorsPerThread( - threadSelectors: ThreadSelectorsPerThread, + threadSelectors: ThreadAndMarkerSelectorsPerThread, threadIndexes: Set, threadsKey: ThreadsKey ) { diff --git a/src/selectors/per-thread/thread.js b/src/selectors/per-thread/thread.js index 0d4da59e77..721832aac7 100644 --- a/src/selectors/per-thread/thread.js +++ b/src/selectors/per-thread/thread.js @@ -44,24 +44,30 @@ import type { import type { UniqueStringArray } from '../../utils/unique-string-array'; import type { TransformLabeL10nIds } from 'firefox-profiler/profile-logic/transforms'; +import type { MarkerSelectorsPerThread } from './markers'; import { mergeThreads } from '../../profile-logic/merge-compare'; import { defaultThreadViewOptions } from '../../reducers/profile-view'; /** - * Infer the return type from the getThreadSelectorsPerThread function. This - * is done that so that the local type definition with `Selector` is the canonical - * definition for the type of the selector. + * Infer the return type from the getBasicThreadSelectorsPerThread and + * getThreadSelectorsWithMarkersPerThread functions. This is done that so that + * the local type definition with `Selector` is the canonical definition for + * the type of the selector. */ -export type ThreadSelectorsPerThread = $ReturnType< - typeof getThreadSelectorsPerThread +export type BasicThreadSelectorsPerThread = $ReturnType< + typeof getBasicThreadSelectorsPerThread >; +export type ThreadSelectorsPerThread = {| + ...BasicThreadSelectorsPerThread, + ...$ReturnType, +|}; /** * Create the selectors for a thread that have to do with an entire thread. This includes * the general filtering pipeline for threads. */ -export function getThreadSelectorsPerThread( +export function getBasicThreadSelectorsPerThread( threadIndexes: Set, threadsKey: ThreadsKey ) { @@ -172,74 +178,6 @@ export function getThreadSelectorsPerThread( } ); - // It becomes very expensive to apply each transform over and over again as they - // typically take around 100ms to run per transform on a fast machine. Memoize - // memoize each step individually so that they transform stack can be pushed and - // popped frequently and easily. - const _applyTransformMemoized = memoize(Transforms.applyTransform, { - cache: new MixedTupleMap(), - }); - - const getTransformStack: Selector = (state) => - UrlState.getTransformStack(state, threadsKey); - - const getRangeAndTransformFilteredThread: Selector = createSelector( - getRangeFilteredThread, - getTransformStack, - ProfileSelectors.getDefaultCategory, - (startingThread, transforms, defaultCategory) => { - return transforms.reduce( - // Apply the reducer using an arrow function to ensure correct memoization. - (thread, transform) => - _applyTransformMemoized(thread, transform, defaultCategory), - startingThread - ); - } - ); - - const _getImplementationFilteredThread: Selector = createSelector( - getRangeAndTransformFilteredThread, - UrlState.getImplementationFilter, - ProfileSelectors.getDefaultCategory, - ProfileData.filterThreadByImplementation - ); - - const _getImplementationAndSearchFilteredThread: Selector = - createSelector( - _getImplementationFilteredThread, - UrlState.getSearchStrings, - (thread, searchStrings) => { - return ProfileData.filterThreadToSearchStrings(thread, searchStrings); - } - ); - - const getFilteredThread: Selector = createSelector( - _getImplementationAndSearchFilteredThread, - UrlState.getInvertCallstack, - ProfileSelectors.getDefaultCategory, - (thread, shouldInvertCallstack, defaultCategory) => { - return shouldInvertCallstack - ? ProfileData.invertCallstack(thread, defaultCategory) - : thread; - } - ); - - const getPreviewFilteredThread: Selector = createSelector( - getFilteredThread, - ProfileSelectors.getPreviewSelection, - (thread, previewSelection): Thread => { - if (!previewSelection.hasSelection) { - return thread; - } - const { selectionStart, selectionEnd } = previewSelection; - return ProfileData.filterThreadSamplesToRange( - thread, - selectionStart, - selectionEnd - ); - } - ); - /** * The CallTreeSummaryStrategy determines how the call tree summarizes the * the current thread. By default, this is done by timing, but other @@ -295,20 +233,6 @@ export function getThreadSelectorsPerThread( CallTree.extractSamplesLikeTable ); - const getFilteredSamplesForCallTree: Selector = - createSelector( - getFilteredThread, - getCallTreeSummaryStrategy, - CallTree.extractSamplesLikeTable - ); - - const getPreviewFilteredSamplesForCallTree: Selector = - createSelector( - getPreviewFilteredThread, - getCallTreeSummaryStrategy, - CallTree.extractSamplesLikeTable - ); - /** * This selector returns the offset to add to a sampleIndex when accessing the * base thread, if your thread is a range filtered thread (all but the base @@ -328,29 +252,6 @@ export function getThreadSelectorsPerThread( } ); - /** - * This selector returns the offset to add to a sampleIndex when accessing the - * base thread, if your thread is the preview filtered thread. - */ - const getSampleIndexOffsetFromPreviewRange: Selector = createSelector( - getFilteredSamplesForCallTree, - ProfileSelectors.getPreviewSelection, - getSampleIndexOffsetFromCommittedRange, - (samples, previewSelection, sampleIndexFromCommittedRange) => { - if (!previewSelection.hasSelection) { - return sampleIndexFromCommittedRange; - } - - const [beginSampleIndex] = ProfileData.getSampleIndexRangeForSelection( - samples, - previewSelection.selectionStart, - previewSelection.selectionEnd - ); - - return sampleIndexFromCommittedRange + beginSampleIndex; - } - ); - const getFriendlyThreadName: Selector = createSelector( ProfileSelectors.getThreads, getThread, @@ -363,27 +264,6 @@ export function getThreadSelectorsPerThread( ProfileData.getThreadProcessDetails ); - const getTransformLabelL10nIds: Selector = - createSelector( - ProfileSelectors.getMeta, - getRangeAndTransformFilteredThread, - getFriendlyThreadName, - getTransformStack, - Transforms.getTransformLabelL10nIds - ); - - const getLocalizedTransformLabels: Selector = createSelector( - getTransformLabelL10nIds, - (transformL10nIds) => - transformL10nIds.map((transform) => ( - - )) - ); - const getViewOptions: Selector = (state) => ProfileSelectors.getProfileViewOptions(state).perThread[threadsKey] || defaultThreadViewOptions; @@ -478,20 +358,11 @@ export function getThreadSelectorsPerThread( getNativeAllocations, getJsAllocations, getThreadRange, - getFilteredThread, getRangeFilteredThread, - getRangeAndTransformFilteredThread, - getPreviewFilteredThread, getUnfilteredSamplesForCallTree, - getFilteredSamplesForCallTree, - getPreviewFilteredSamplesForCallTree, getSampleIndexOffsetFromCommittedRange, - getSampleIndexOffsetFromPreviewRange, getFriendlyThreadName, getThreadProcessDetails, - getTransformLabelL10nIds, - getLocalizedTransformLabels, - getTransformStack, getViewOptions, getJsTracerTable, getExpensiveJsTracerTiming, @@ -507,3 +378,152 @@ export function getThreadSelectorsPerThread( getCallTreeSummaryStrategy, }; } + +type BasicThreadAndMarkerSelectorsPerThread = {| + ...BasicThreadSelectorsPerThread, + ...MarkerSelectorsPerThread, +|}; + +export function getThreadSelectorsWithMarkersPerThread( + threadSelectors: BasicThreadAndMarkerSelectorsPerThread, + threadIndexes: Set, + threadsKey: ThreadsKey +) { + // It becomes very expensive to apply each transform over and over again as they + // typically take around 100ms to run per transform on a fast machine. Memoize + // memoize each step individually so that they transform stack can be pushed and + // popped frequently and easily. + const _applyTransformMemoized = memoize(Transforms.applyTransform, { + cache: new MixedTupleMap(), + }); + + const getTransformStack: Selector = (state) => + UrlState.getTransformStack(state, threadsKey); + + const getRangeAndTransformFilteredThread: Selector = createSelector( + threadSelectors.getRangeFilteredThread, + getTransformStack, + ProfileSelectors.getDefaultCategory, + (startingThread, transforms, defaultCategory) => { + return transforms.reduce( + // Apply the reducer using an arrow function to ensure correct memoization. + (thread, transform) => + _applyTransformMemoized(thread, transform, defaultCategory), + startingThread + ); + } + ); + + const _getImplementationFilteredThread: Selector = createSelector( + getRangeAndTransformFilteredThread, + UrlState.getImplementationFilter, + ProfileSelectors.getDefaultCategory, + ProfileData.filterThreadByImplementation + ); + + const _getImplementationAndSearchFilteredThread: Selector = + createSelector( + _getImplementationFilteredThread, + UrlState.getSearchStrings, + (thread, searchStrings) => { + return ProfileData.filterThreadToSearchStrings(thread, searchStrings); + } + ); + + const getFilteredThread: Selector = createSelector( + _getImplementationAndSearchFilteredThread, + UrlState.getInvertCallstack, + ProfileSelectors.getDefaultCategory, + (thread, shouldInvertCallstack, defaultCategory) => { + return shouldInvertCallstack + ? ProfileData.invertCallstack(thread, defaultCategory) + : thread; + } + ); + + const getPreviewFilteredThread: Selector = createSelector( + getFilteredThread, + ProfileSelectors.getPreviewSelection, + (thread, previewSelection): Thread => { + if (!previewSelection.hasSelection) { + return thread; + } + const { selectionStart, selectionEnd } = previewSelection; + return ProfileData.filterThreadSamplesToRange( + thread, + selectionStart, + selectionEnd + ); + } + ); + + const getFilteredSamplesForCallTree: Selector = + createSelector( + getFilteredThread, + threadSelectors.getCallTreeSummaryStrategy, + CallTree.extractSamplesLikeTable + ); + + const getPreviewFilteredSamplesForCallTree: Selector = + createSelector( + getPreviewFilteredThread, + threadSelectors.getCallTreeSummaryStrategy, + CallTree.extractSamplesLikeTable + ); + + /** + * This selector returns the offset to add to a sampleIndex when accessing the + * base thread, if your thread is the preview filtered thread. + */ + const getSampleIndexOffsetFromPreviewRange: Selector = createSelector( + getFilteredSamplesForCallTree, + ProfileSelectors.getPreviewSelection, + threadSelectors.getSampleIndexOffsetFromCommittedRange, + (samples, previewSelection, sampleIndexFromCommittedRange) => { + if (!previewSelection.hasSelection) { + return sampleIndexFromCommittedRange; + } + + const [beginSampleIndex] = ProfileData.getSampleIndexRangeForSelection( + samples, + previewSelection.selectionStart, + previewSelection.selectionEnd + ); + + return sampleIndexFromCommittedRange + beginSampleIndex; + } + ); + + const getTransformLabelL10nIds: Selector = + createSelector( + ProfileSelectors.getMeta, + getRangeAndTransformFilteredThread, + threadSelectors.getFriendlyThreadName, + getTransformStack, + Transforms.getTransformLabelL10nIds + ); + + const getLocalizedTransformLabels: Selector = createSelector( + getTransformLabelL10nIds, + (transformL10nIds) => + transformL10nIds.map((transform) => ( + + )) + ); + + return { + getTransformStack, + getRangeAndTransformFilteredThread, + getFilteredThread, + getPreviewFilteredThread, + getFilteredSamplesForCallTree, + getPreviewFilteredSamplesForCallTree, + getSampleIndexOffsetFromPreviewRange, + getTransformLabelL10nIds, + getLocalizedTransformLabels, + }; +} From 9fb0eae3b901e638607c40e35788bfa5444654c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Mon, 8 May 2023 15:44:07 +0200 Subject: [PATCH 02/10] Add a new filter-samples transform that can filter samples by marker search filter --- locales/en-US/app.ftl | 5 + src/components/shared/CallNodeContextMenu.js | 4 + src/profile-logic/transforms.js | 184 ++++++++++++++++++- src/selectors/per-thread/thread.js | 24 ++- src/types/transforms.js | 19 ++ src/utils/flow.js | 1 + 6 files changed, 234 insertions(+), 3 deletions(-) diff --git a/locales/en-US/app.ftl b/locales/en-US/app.ftl index 1e01f06651..25033c362e 100644 --- a/locales/en-US/app.ftl +++ b/locales/en-US/app.ftl @@ -972,6 +972,11 @@ TransformNavigator--collapse-direct-recursion-only = Collapse direct recursion o # $item (String) - Name of the function that transform applied to. TransformNavigator--collapse-function-subtree = Collapse subtree: { $item } +# "Drop samples outside of filtered markers" transform. +# Variables: +# $item (String) - Search filter of the markers that transform will apply to. +TransformNavigator--drop-samples-outside-of-marker-filter = Drop samples outside of marker filter: “{ $item }” + ## "Bottom box" - a view which contains the source view and the assembly view, ## at the bottom of the profiler UI ## diff --git a/src/components/shared/CallNodeContextMenu.js b/src/components/shared/CallNodeContextMenu.js index 8bfba53344..2b747b40d0 100644 --- a/src/components/shared/CallNodeContextMenu.js +++ b/src/components/shared/CallNodeContextMenu.js @@ -363,6 +363,10 @@ class CallNodeContextMenuImpl extends React.PureComponent { }); break; } + case 'filter-samples': + throw new Error( + "Filter samples transform can't be applied from the call node context menu." + ); default: assertExhaustiveCheck(type); } diff --git a/src/profile-logic/transforms.js b/src/profile-logic/transforms.js index 62bd2f7a16..a9c94f9913 100644 --- a/src/profile-logic/transforms.js +++ b/src/profile-logic/transforms.js @@ -17,12 +17,14 @@ import { import { timeCode } from '../utils/time-code'; import { assertExhaustiveCheck, convertToTransformType } from '../utils/flow'; import { CallTree } from '../profile-logic/call-tree'; +import { getSearchFilteredMarkerIndexes } from '../profile-logic/marker-data'; import { shallowCloneFrameTable, shallowCloneFuncTable, getEmptyStackTable, } from './data-structures'; import { getFunctionName } from './function-info'; +import { splitSearchString, stringsToRegExp } from '../utils/string'; import type { Thread, @@ -40,6 +42,12 @@ import type { TransformType, TransformStack, ProfileMeta, + StartEndRange, + FilterSamplesType, + Marker, + MarkerIndex, + MarkerSchemaByName, + CategoryList, } from 'firefox-profiler/types'; /** @@ -61,6 +69,7 @@ const SHORT_KEY_TO_TRANSFORM: { [string]: TransformType } = {}; 'collapse-direct-recursion', 'collapse-recursion', 'collapse-function-subtree', + 'filter-samples', ].forEach((transform: TransformType) => { // This is kind of an awkward switch, but it ensures we've exhaustively checked that // we have a mapping for every transform. @@ -96,6 +105,9 @@ const SHORT_KEY_TO_TRANSFORM: { [string]: TransformType } = {}; case 'collapse-function-subtree': shortKey = 'cfs'; break; + case 'filter-samples': + shortKey = 'fs'; + break; default: { throw assertExhaustiveCheck(transform); } @@ -268,6 +280,20 @@ export function parseTransforms(transformString: string): TransformStack { break; } + case 'filter-samples': { + // e.g. "fs-m-BackboneJS-TodoMVC.Adding100Items-async" + const [, shortFilterType, ...filter] = tuple; + // Filter string may include "-" characters, so we need to join them back. + const filterString = filter.join('-'); + const filterType = convertToFullFilterType(shortFilterType); + + transforms.push({ + type: 'filter-samples', + filterType, + filter: filterString, + }); + break; + } default: throw assertExhaustiveCheck(type); } @@ -275,6 +301,30 @@ export function parseTransforms(transformString: string): TransformStack { return transforms; } +/** + * Convert the shortened filter type into the full filter type. + */ +function convertToFullFilterType(shortFilterType: string): FilterSamplesType { + switch (shortFilterType) { + case 'm': + return 'marker-search'; + default: + throw new Error('Unknown filter type.'); + } +} + +/** + * Convert the full filter type into the shortened filter type. + */ +function convertToShortFilterType(filterType: FilterSamplesType): string { + switch (filterType) { + case 'marker-search': + return 'm'; + default: + throw assertExhaustiveCheck(filterType); + } +} + /** * Each transform in the stack is separated by a "~". */ @@ -317,6 +367,10 @@ export function stringifyTransforms(transformStack: TransformStack): string { } return string; } + case 'filter-samples': + return `${shortKey}-${convertToShortFilterType( + transform.filterType + )}-${transform.filter}`; default: throw assertExhaustiveCheck(transform); } @@ -364,6 +418,18 @@ export function getTransformLabelL10nIds( }; } + if (transform.type === 'filter-samples') { + switch (transform.filterType) { + case 'marker-search': + return { + l10nId: 'TransformNavigator--drop-samples-outside-of-marker-filter', + item: transform.filter, + }; + default: + throw assertExhaustiveCheck(transform.filterType); + } + } + // Lookup function name. let funcIndex; switch (transform.type) { @@ -474,6 +540,15 @@ export function applyTransformToCallNodePath( transform.funcIndex, callNodePath ); + case 'filter-samples': + // There's nothing to update in the call node path. But this call node path + // could disappear if we filtered out all the samples with this path. + // This is also the case for drop-function transform. We need to have a + // generic mechanism for: if the selected call node (after the transformation + // has been applied to the call path) is not present in the call tree, run + // some generic code that finds a close-by call node which is present. + // See: https://github.com/firefox-devtools/profiler/issues/4618 + return callNodePath; default: throw assertExhaustiveCheck(transform); } @@ -1674,10 +1749,107 @@ export function funcHasRecursiveCall( return false; } +function _findRangesByMarkerFilter( + getMarker: (MarkerIndex) => Marker, + markerIndexes: MarkerIndex[], + markerSchemaByName: MarkerSchemaByName, + categoryList: CategoryList, + filter: string +): StartEndRange[] { + const ranges = []; + + const searchRegExp = stringsToRegExp(splitSearchString(filter)); + const searchFilteredMarkerIndexes = getSearchFilteredMarkerIndexes( + getMarker, + markerIndexes, + markerSchemaByName, + searchRegExp, + categoryList + ); + + for (const markerIndex of searchFilteredMarkerIndexes) { + const { start, end } = getMarker(markerIndex); + + if (start === null || end === null) { + // This is not an interval marker, so we can't use it as a range. + continue; + } + + ranges.push({ start: start, end: end }); + } + return ranges; +} + +/** + * Find the sample ranges to filter depending on the filter type, then go + * through all the samples and remove the ones that are outside of the ranges. + */ +export function filterSamples( + thread: Thread, + getMarker: (MarkerIndex) => Marker, + markerIndexes: MarkerIndex[], + markerSchemaByName: MarkerSchemaByName, + categoryList: CategoryList, + filterType: FilterSamplesType, + filter: string +): Thread { + return timeCode('filterSamples', () => { + // Find the ranges to filter. + let ranges: StartEndRange[]; + switch (filterType) { + case 'marker-search': + ranges = _findRangesByMarkerFilter( + getMarker, + markerIndexes, + markerSchemaByName, + categoryList, + filter + ); + break; + default: + throw assertExhaustiveCheck(filterType); + } + + // Now let's go through all the samples and remove the ones that are outside + // of the ranges. + const { samples } = thread; + const newSamples = { + ...samples, + stack: samples.stack.slice(), + }; + + for (let sampleIndex = 0; sampleIndex < samples.length; sampleIndex++) { + const sampleTime = samples.time[sampleIndex]; + + let sampleInRange = false; + for (const { start, end } of ranges) { + if (sampleTime >= start && sampleTime <= end) { + sampleInRange = true; + break; + } + } + + if (!sampleInRange) { + newSamples.stack[sampleIndex] = null; + } + } + + const newThread = { + ...thread, + samples: newSamples, + }; + return newThread; + }); +} + export function applyTransform( thread: Thread, transform: Transform, - defaultCategory: IndexIntoCategoryList + defaultCategory: IndexIntoCategoryList, + getMarker: (MarkerIndex) => Marker, + markerIndexes: MarkerIndex[], + markerSchemaByName: MarkerSchemaByName, + categoryList: CategoryList ): Thread { switch (transform.type) { case 'focus-subtree': @@ -1727,6 +1899,16 @@ export function applyTransform( transform.funcIndex, defaultCategory ); + case 'filter-samples': + return filterSamples( + thread, + getMarker, + markerIndexes, + markerSchemaByName, + categoryList, + transform.filterType, + transform.filter + ); default: throw assertExhaustiveCheck(transform); } diff --git a/src/selectors/per-thread/thread.js b/src/selectors/per-thread/thread.js index 721832aac7..b0ecded1c1 100644 --- a/src/selectors/per-thread/thread.js +++ b/src/selectors/per-thread/thread.js @@ -404,11 +404,31 @@ export function getThreadSelectorsWithMarkersPerThread( threadSelectors.getRangeFilteredThread, getTransformStack, ProfileSelectors.getDefaultCategory, - (startingThread, transforms, defaultCategory) => { + threadSelectors.getMarkerGetter, + threadSelectors.getFullMarkerListIndexes, + ProfileSelectors.getMarkerSchemaByName, + ProfileSelectors.getCategories, + ( + startingThread, + transforms, + defaultCategory, + markerGetter, + markerIndexes, + markerSchemaByName, + categories + ) => { return transforms.reduce( // Apply the reducer using an arrow function to ensure correct memoization. (thread, transform) => - _applyTransformMemoized(thread, transform, defaultCategory), + _applyTransformMemoized( + thread, + transform, + defaultCategory, + markerGetter, + markerIndexes, + markerSchemaByName, + categories + ), startingThread ); } diff --git a/src/types/transforms.js b/src/types/transforms.js index 3eca195545..54d267fd95 100644 --- a/src/types/transforms.js +++ b/src/types/transforms.js @@ -24,6 +24,13 @@ import type { import type { CallNodePath, ThreadsKey } from './profile-derived'; import type { ImplementationFilter } from './actions'; +/** + * This type represents the filter types for the 'filter-samples' transform. + * Currently the only filter type is 'marker-search', but in the future we may + * add more types of filters. + */ +export type FilterSamplesType = 'marker-search'; + /* * Define all of the transforms on an object to conveniently access $ObjMap and do * nice things like iterate over every transform type. There is no way to create a @@ -325,6 +332,18 @@ export type TransformDefinitions = { +type: 'focus-category', +category: IndexIntoCategoryList, |}, + + /** + * Filter the samples in the thread by the filter. + * Currently it only supports filtering by the marker name but can be extended + * to support more filters in the future. + */ + 'filter-samples': {| + +type: 'filter-samples', + // Expand this type when you need to support more than just the marker. + +filterType: FilterSamplesType, + +filter: string, + |}, }; // Extract the transforms into a union. diff --git a/src/utils/flow.js b/src/utils/flow.js index 6980f360ca..1f6e834b3c 100644 --- a/src/utils/flow.js +++ b/src/utils/flow.js @@ -93,6 +93,7 @@ export function convertToTransformType(type: string): TransformType | null { case 'collapse-recursion': case 'collapse-function-subtree': case 'drop-function': + case 'filter-samples': return coercedType; default: { // The coerced type SHOULD be empty here. If in reality we get From 2837d2cee6137c8b6455e20c8f6c85117cdeced8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Wed, 24 May 2023 14:00:42 +0200 Subject: [PATCH 03/10] Add a new marker filters menu and add a menu item for dropping samples --- locales/en-US/app.ftl | 10 +++ res/img/svg/filter.svg | 7 ++ .../shared/MarkerFiltersContextMenu.js | 73 +++++++++++++++++++ src/components/shared/MarkerSettings.css | 10 +++ src/components/shared/MarkerSettings.js | 32 ++++++++ .../__snapshots__/MarkerChart.test.js.snap | 42 +++++++++++ .../__snapshots__/MarkerTable.test.js.snap | 21 ++++++ 7 files changed, 195 insertions(+) create mode 100644 res/img/svg/filter.svg create mode 100644 src/components/shared/MarkerFiltersContextMenu.js diff --git a/locales/en-US/app.ftl b/locales/en-US/app.ftl index 25033c362e..ed96eaf30d 100644 --- a/locales/en-US/app.ftl +++ b/locales/en-US/app.ftl @@ -397,6 +397,13 @@ MarkerContextMenu--select-the-receiver-thread = MarkerContextMenu--select-the-sender-thread = Select the sender thread “{ $threadName }” +## MarkerFiltersContextMenu +## This is the menu when filter icon is clicked in Marker Chart and Marker Table +## panels. + +MarkerFiltersContextMenu--drop-samples-outside-of-filtered-markers = + Drop samples outside of filtered markers + ## MarkerSettings ## This is used in all panels related to markers. @@ -404,6 +411,9 @@ MarkerSettings--panel-search = .label = Filter Markers: .title = Only display markers that match a certain name +MarkerSettings--marker-filters = + .title = Marker Filters + ## MarkerSidebar ## This is the sidebar component that is used in Marker Table panel. diff --git a/res/img/svg/filter.svg b/res/img/svg/filter.svg new file mode 100644 index 0000000000..94f1f25c45 --- /dev/null +++ b/res/img/svg/filter.svg @@ -0,0 +1,7 @@ + + + + + diff --git a/src/components/shared/MarkerFiltersContextMenu.js b/src/components/shared/MarkerFiltersContextMenu.js new file mode 100644 index 0000000000..aac51a51c0 --- /dev/null +++ b/src/components/shared/MarkerFiltersContextMenu.js @@ -0,0 +1,73 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +// @flow +import React, { PureComponent } from 'react'; +import { MenuItem } from '@firefox-devtools/react-contextmenu'; +import { Localized } from '@fluent/react'; + +import { ContextMenu } from './ContextMenu'; +import explicitConnect from 'firefox-profiler/utils/connect'; +import { + getMarkersSearchString, + getSelectedThreadsKey, +} from 'firefox-profiler/selectors/url-state'; +import { addTransformToStack } from 'firefox-profiler/actions/profile-view'; + +import type { ThreadsKey } from 'firefox-profiler/types'; +import type { ConnectedProps } from 'firefox-profiler/utils/connect'; + +type StateProps = {| + +searchString: string, + +threadsKey: ThreadsKey, +|}; + +type DispatchProps = {| + +addTransformToStack: typeof addTransformToStack, +|}; + +type Props = ConnectedProps<{||}, StateProps, DispatchProps>; + +class MarkerFiltersContextMenuImpl extends PureComponent { + filterSamplesByMarker = () => { + const { searchString, threadsKey, addTransformToStack } = this.props; + addTransformToStack(threadsKey, { + type: 'filter-samples', + filterType: 'marker-search', + filter: searchString, + }); + }; + + render() { + return ( + + + + Drop samples outside of filtered markers + + + + ); + } +} + +export const MarkerFiltersContextMenu = explicitConnect< + {||}, + StateProps, + DispatchProps +>({ + mapStateToProps: (state) => { + return { + searchString: getMarkersSearchString(state), + threadsKey: getSelectedThreadsKey(state), + }; + }, + mapDispatchToProps: { + addTransformToStack, + }, + component: MarkerFiltersContextMenuImpl, +}); diff --git a/src/components/shared/MarkerSettings.css b/src/components/shared/MarkerSettings.css index 43c3bbf944..dd964fd0ca 100644 --- a/src/components/shared/MarkerSettings.css +++ b/src/components/shared/MarkerSettings.css @@ -9,3 +9,13 @@ padding: 0; line-height: 25px; } + +.filterMarkersButton { + width: 24px; + height: 24px; + flex: none; + margin: 0 4px; + background-image: url(firefox-profiler-res/img/svg/filter.svg); + background-position: center; + background-repeat: no-repeat; +} diff --git a/src/components/shared/MarkerSettings.js b/src/components/shared/MarkerSettings.js index 50296f5cef..5c6283a68f 100644 --- a/src/components/shared/MarkerSettings.js +++ b/src/components/shared/MarkerSettings.js @@ -6,12 +6,15 @@ import React, { PureComponent } from 'react'; import { Localized } from '@fluent/react'; +import classNames from 'classnames'; +import { showMenu } from '@firefox-devtools/react-contextmenu'; import explicitConnect from 'firefox-profiler/utils/connect'; import { changeMarkersSearchString } from 'firefox-profiler/actions/profile-view'; import { getMarkersSearchString } from 'firefox-profiler/selectors/url-state'; import { PanelSearch } from './PanelSearch'; import { StackImplementationSetting } from 'firefox-profiler/components/shared/StackImplementationSetting'; +import { MarkerFiltersContextMenu } from './MarkerFiltersContextMenu'; import type { ConnectedProps } from 'firefox-profiler/utils/connect'; @@ -33,6 +36,21 @@ class MarkerSettingsImpl extends PureComponent { this.props.changeMarkersSearchString(value); }; + _onClickFilterButton = (event: SyntheticMouseEvent) => { + const rect = event.currentTarget.getBoundingClientRect(); + // FIXME: Currently we assume that the context menu is 250px wide, but ideally + // we should get the real width. It's not so easy though, because the context + // menu is not rendered yet. + const isRightAligned = rect.right > window.innerWidth - 250; + + showMenu({ + data: null, + id: 'MarkerFiltersContextMenu', + position: { x: isRightAligned ? rect.right : rect.left, y: rect.bottom }, + target: event.target, + }); + }; + render() { const { searchString } = this.props; return ( @@ -52,6 +70,20 @@ class MarkerSettingsImpl extends PureComponent { onSearch={this._onSearch} /> + +