From b3c31ba15b0c311a73a85be23ee88eddba2e8b35 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 1/9] 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 3aafc3ef2c659ef227579754387c6ad790639c6e 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 2/9] Add a new filter-samples transform that can filter samples by marker name --- locales/en-US/app.ftl | 5 + src/components/shared/CallNodeContextMenu.js | 4 + src/profile-logic/transforms.js | 153 ++++++++++++++++++- src/selectors/per-thread/thread.js | 5 +- src/types/transforms.js | 19 +++ src/utils/flow.js | 1 + 6 files changed, 184 insertions(+), 3 deletions(-) diff --git a/locales/en-US/app.ftl b/locales/en-US/app.ftl index 62977a680a..8441afb05c 100644 --- a/locales/en-US/app.ftl +++ b/locales/en-US/app.ftl @@ -950,6 +950,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 markers" transform. +# Variables: +# $item (String) - Name of the markers that transform will filter to. +TransformNavigator--drop-samples-outside-of-markers = Drop samples outside of markers: { $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..7b1e1448cd 100644 --- a/src/profile-logic/transforms.js +++ b/src/profile-logic/transforms.js @@ -40,6 +40,9 @@ import type { TransformType, TransformStack, ProfileMeta, + StartEndRange, + FilterSamplesType, + Marker, } from 'firefox-profiler/types'; /** @@ -61,6 +64,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 +100,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 +275,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 +296,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'; + 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': + return 'm'; + default: + throw assertExhaustiveCheck(filterType); + } +} + /** * Each transform in the stack is separated by a "~". */ @@ -317,6 +362,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 +413,18 @@ export function getTransformLabelL10nIds( }; } + if (transform.type === 'filter-samples') { + switch (transform.filterType) { + case 'marker': + return { + l10nId: 'TransformNavigator--drop-samples-outside-of-markers', + item: transform.filter, + }; + default: + throw assertExhaustiveCheck(transform.filterType); + } + } + // Lookup function name. let funcIndex; switch (transform.type) { @@ -474,6 +535,14 @@ 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. + return callNodePath; default: throw assertExhaustiveCheck(transform); } @@ -1674,10 +1743,85 @@ export function funcHasRecursiveCall( return false; } +function _findRangesByMarkerFilter( + markers: Marker[], + filter: string +): StartEndRange[] { + const ranges = []; + + for (let markerIndex = 0; markerIndex < markers.length; markerIndex++) { + const { name, start, end } = markers[markerIndex]; + + if (start === null || end === null) { + // This is not an interval marker, so we can't use it as a range. + continue; + } + + if (name === filter) { + 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, + markers: Marker[], + filterType: FilterSamplesType, + filter: string +): Thread { + return timeCode('filterSamples', () => { + // Find the ranges to filter. + let ranges: StartEndRange[]; + switch (filterType) { + case 'marker': + ranges = _findRangesByMarkerFilter(markers, 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, + markers: Marker[] ): Thread { switch (transform.type) { case 'focus-subtree': @@ -1727,6 +1871,13 @@ export function applyTransform( transform.funcIndex, defaultCategory ); + case 'filter-samples': + return filterSamples( + thread, + markers, + 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..76bae8b963 100644 --- a/src/selectors/per-thread/thread.js +++ b/src/selectors/per-thread/thread.js @@ -404,11 +404,12 @@ export function getThreadSelectorsWithMarkersPerThread( threadSelectors.getRangeFilteredThread, getTransformStack, ProfileSelectors.getDefaultCategory, - (startingThread, transforms, defaultCategory) => { + threadSelectors.getFullMarkerList, + (startingThread, transforms, defaultCategory, markers) => { return transforms.reduce( // Apply the reducer using an arrow function to ensure correct memoization. (thread, transform) => - _applyTransformMemoized(thread, transform, defaultCategory), + _applyTransformMemoized(thread, transform, defaultCategory, markers), startingThread ); } diff --git a/src/types/transforms.js b/src/types/transforms.js index 3eca195545..cdad5aeef3 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', but in the future we may add + * more types of filters. + */ +export type FilterSamplesType = 'marker'; + /* * 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 382740dd84679a5a706db297646fd517743bfb2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Mon, 8 May 2023 15:45:58 +0200 Subject: [PATCH 3/9] Add a marker context menu item to filter samples by marker name --- locales/en-US/app.ftl | 6 +++ src/components/shared/MarkerContextMenu.js | 49 +++++++++++++++++++ src/test/components/MarkerTable.test.js | 3 +- .../__snapshots__/MarkerChart.test.js.snap | 18 +++++++ 4 files changed, 75 insertions(+), 1 deletion(-) diff --git a/locales/en-US/app.ftl b/locales/en-US/app.ftl index 8441afb05c..ae24cd91d3 100644 --- a/locales/en-US/app.ftl +++ b/locales/en-US/app.ftl @@ -383,6 +383,12 @@ MarkerContextMenu--copy-url = Copy URL MarkerContextMenu--copy-page-url = Copy page URL MarkerContextMenu--copy-as-json = Copy as JSON +# This string is used on the marker context menu item when right clicked on any marker. +# It drops the samples outside of given markers. +# Variables: +# $markerName (String) - Name of the marker that is selected. +MarkerContextMenu--drop-samples-outside-of-marker = Drop samples outside of “{ $markerName }” markers + # This string is used on the marker context menu item when right clicked on an # IPC marker. # Variables: diff --git a/src/components/shared/MarkerContextMenu.js b/src/components/shared/MarkerContextMenu.js index a6318580c2..125d664ac8 100644 --- a/src/components/shared/MarkerContextMenu.js +++ b/src/components/shared/MarkerContextMenu.js @@ -15,7 +15,9 @@ import { setContextMenuVisibility, updatePreviewSelection, selectTrackFromTid, + addTransformToStack, } from 'firefox-profiler/actions/profile-view'; +import { changeSelectedTab } from 'firefox-profiler/actions/app'; import { getPreviewSelection, getCommittedRange, @@ -71,6 +73,8 @@ type DispatchProps = {| +updatePreviewSelection: typeof updatePreviewSelection, +setContextMenuVisibility: typeof setContextMenuVisibility, +selectTrackFromTid: typeof selectTrackFromTid, + +addTransformToStack: typeof addTransformToStack, + +changeSelectedTab: typeof changeSelectedTab, |}; type Props = ConnectedProps; @@ -182,6 +186,24 @@ class MarkerContextMenuImpl extends PureComponent { copy(getMarkerLabelToCopy(markerIndex)); }; + filterByMarkerName = () => { + const { + rightClickedMarkerInfo, + marker, + addTransformToStack, + changeSelectedTab, + } = this.props; + const { threadsKey } = rightClickedMarkerInfo; + addTransformToStack(threadsKey, { + type: 'filter-samples', + filterType: 'marker', + filter: marker.name, + }); + + // Then change the selected tab to call tree. + changeSelectedTab('calltree'); + }; + copyMarkerCause = () => { const { marker } = this.props; @@ -479,7 +501,32 @@ class MarkerContextMenuImpl extends PureComponent { )} + {/* Only show this context menu item for interval markers */} + {marker.start !== null && marker.end !== null ? ( + <> +
+ + + {/* TODO: Find an icon. */} + + }} + > + <> + Drop samples outside of “{marker.name}” + markers + + + + + ) : null} +
+ @@ -536,6 +583,8 @@ const MarkerContextMenu = explicitConnect({ updatePreviewSelection, setContextMenuVisibility, selectTrackFromTid, + addTransformToStack, + changeSelectedTab, }, component: MarkerContextMenuImpl, }); diff --git a/src/test/components/MarkerTable.test.js b/src/test/components/MarkerTable.test.js index e1ef7be42e..41ea969795 100644 --- a/src/test/components/MarkerTable.test.js +++ b/src/test/components/MarkerTable.test.js @@ -141,7 +141,8 @@ describe('MarkerTable', function () { fireFullContextMenu(getByText(/setTimeout/)); checkMenuIsDisplayedForNode(/setTimeout/); - expect(getRowElement(/setTimeout/)).toHaveClass('isRightClicked'); + // There is another text with setTimeout inside the context menu. Pick the right one. + expect(getRowElement(/setTimeout\s/)).toHaveClass('isRightClicked'); // Wait that all timers are done before trying again. jest.runAllTimers(); diff --git a/src/test/components/__snapshots__/MarkerChart.test.js.snap b/src/test/components/__snapshots__/MarkerChart.test.js.snap index 199cb5b4af..8ca510c837 100644 --- a/src/test/components/__snapshots__/MarkerChart.test.js.snap +++ b/src/test/components/__snapshots__/MarkerChart.test.js.snap @@ -133,6 +133,24 @@ exports[`MarkerChart context menus displays when right clicking on a marker 1`]
+ +
Date: Mon, 8 May 2023 17:31:37 +0200 Subject: [PATCH 4/9] Add a filter icon for the context menu item --- res/img/svg/filter.svg | 7 +++++++ src/components/shared/MarkerContextMenu.css | 4 ++++ src/components/shared/MarkerContextMenu.js | 3 +-- src/test/components/__snapshots__/MarkerChart.test.js.snap | 2 +- 4 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 res/img/svg/filter.svg 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/MarkerContextMenu.css b/src/components/shared/MarkerContextMenu.css index 22d06d6859..bfafc741b2 100644 --- a/src/components/shared/MarkerContextMenu.css +++ b/src/components/shared/MarkerContextMenu.css @@ -51,6 +51,10 @@ background-image: url(firefox-profiler-res/img/svg/select-thread.svg); } +.markerContextMenuIconFilterSamples { + background-image: url(firefox-profiler-res/img/svg/filter.svg); +} + /* These icons don't look nice when selected, unless if we invert them. However * the other icons look better without the filter, so we don't change them in * this state. */ diff --git a/src/components/shared/MarkerContextMenu.js b/src/components/shared/MarkerContextMenu.js index 125d664ac8..d781503de2 100644 --- a/src/components/shared/MarkerContextMenu.js +++ b/src/components/shared/MarkerContextMenu.js @@ -507,8 +507,7 @@ class MarkerContextMenuImpl extends PureComponent {
- {/* TODO: Find an icon. */} - + Drop samples outside of “ From d3361d7e41e6b47a8811ffcbbb33b39f0b4bb050 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Mon, 8 May 2023 20:48:11 +0200 Subject: [PATCH 5/9] Add tests for "filter-samples" transform --- src/test/store/transforms.test.js | 140 ++++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) diff --git a/src/test/store/transforms.test.js b/src/test/store/transforms.test.js index 3da73a19a6..eb08c7c085 100644 --- a/src/test/store/transforms.test.js +++ b/src/test/store/transforms.test.js @@ -8,6 +8,7 @@ import { getProfileWithUnbalancedNativeAllocations, getProfileWithBalancedNativeAllocations, getProfileWithJsAllocations, + addMarkersToThreadWithCorrespondingSamples, } from '../fixtures/profiles/processed-profile'; import { formatTree } from '../fixtures/utils'; import { storeWithProfile } from '../fixtures/stores'; @@ -1824,6 +1825,145 @@ describe('"collapse-recursion" transform', function () { }); }); +describe('"filter-samples" transform', function () { + describe('on a call tree', function () { + const { profile } = getProfileFromTextSamples(` + A A A A A + B B C B F + C C E + D + `); + const threadIndex = 0; + addMarkersToThreadWithCorrespondingSamples(profile.threads[threadIndex], [ + [ + 'Marker A', + 0, + 0.5, + { + type: 'DOMEvent', + latency: 7, + eventType: 'click', + }, + ], + [ + 'Marker B', + 0.5, + 1.5, + { + type: 'UserTiming', + name: 'measure-1', + entryType: 'measure', + }, + ], + [ + 'Marker C', + 1.5, + 2.5, + { + type: 'UserTiming', + name: 'measure-2', + entryType: 'measure', + }, + ], + [ + 'Marker C', + 2.5, + 3.5, + { + type: 'UserTiming', + name: 'measure-2', + entryType: 'measure', + }, + ], + ]); + + const { dispatch, getState } = storeWithProfile(profile); + const originalCallTree = selectedThreadSelectors.getCallTree(getState()); + + it('starts as an unfiltered call tree', function () { + expect(formatTree(originalCallTree)).toEqual([ + '- A (total: 5, self: —)', + ' - B (total: 3, self: —)', + ' - C (total: 2, self: 1)', + ' - D (total: 1, self: 1)', + ' - E (total: 1, self: 1)', + ' - C (total: 1, self: 1)', + ' - F (total: 1, self: 1)', + ]); + }); + + it('filtered to range of "Marker A"', function () { + dispatch( + addTransformToStack(threadIndex, { + type: 'filter-samples', + filterType: 'marker', + filter: 'Marker A', + }) + ); + const callTree = selectedThreadSelectors.getCallTree(getState()); + expect(formatTree(callTree)).toEqual([ + '- A (total: 1, self: —)', + ' - B (total: 1, self: —)', + ' - C (total: 1, self: 1)', + ]); + // Reset the transform stack. + dispatch(popTransformsFromStack(0)); + }); + + it('filtered to range of "Marker B"', function () { + dispatch( + addTransformToStack(threadIndex, { + type: 'filter-samples', + filterType: 'marker', + filter: 'Marker B', + }) + ); + const callTree = selectedThreadSelectors.getCallTree(getState()); + expect(formatTree(callTree)).toEqual([ + '- A (total: 1, self: —)', + ' - B (total: 1, self: —)', + ' - C (total: 1, self: —)', + ' - D (total: 1, self: 1)', + ]); + // Reset the transform stack. + dispatch(popTransformsFromStack(0)); + }); + + it('filtered to multiple ranges of "Marker C"', function () { + dispatch( + addTransformToStack(threadIndex, { + type: 'filter-samples', + filterType: 'marker', + filter: 'Marker C', + }) + ); + const callTree = selectedThreadSelectors.getCallTree(getState()); + expect(formatTree(callTree)).toEqual([ + '- A (total: 2, self: —)', + ' - B (total: 1, self: —)', + ' - E (total: 1, self: 1)', + ' - C (total: 1, self: 1)', + ]); + // Reset the transform stack. + dispatch(popTransformsFromStack(0)); + }); + + it('filtered to a non-existent marker name', function () { + dispatch( + addTransformToStack(threadIndex, { + type: 'filter-samples', + filterType: 'marker', + filter: 'non-existent', + }) + ); + const callTree = selectedThreadSelectors.getCallTree(getState()); + expect(formatTree(callTree)).toEqual([]); + // Reset the transform stack. + dispatch(popTransformsFromStack(0)); + }); + }); +}); + describe('expanded and selected CallNodePaths', function () { const { profile, From 100dcc8befedd9f31da59279db7f85c631bda825 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Mon, 15 May 2023 10:28:33 -0400 Subject: [PATCH 6/9] Filter js and native allocation stacks as well --- src/profile-logic/transforms.js | 54 +++++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/src/profile-logic/transforms.js b/src/profile-logic/transforms.js index 7b1e1448cd..0a6511ce0c 100644 --- a/src/profile-logic/transforms.js +++ b/src/profile-logic/transforms.js @@ -43,6 +43,7 @@ import type { StartEndRange, FilterSamplesType, Marker, + Milliseconds, } from 'firefox-profiler/types'; /** @@ -1787,32 +1788,53 @@ export function filterSamples( // 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(), - }; + const { samples, jsAllocations, nativeAllocations } = thread; - for (let sampleIndex = 0; sampleIndex < samples.length; sampleIndex++) { - const sampleTime = samples.time[sampleIndex]; + function filterTable< + Table: { + stack: Array, + time: Milliseconds[], + length: number, + } + >(table: Table): Table { + const newTable = { + ...table, + stack: table.stack.slice(), + }; - let sampleInRange = false; - for (const { start, end } of ranges) { - if (sampleTime >= start && sampleTime <= end) { - sampleInRange = true; - break; + for (let tableIndex = 0; tableIndex < newTable.length; tableIndex++) { + const sampleTime = newTable.time[tableIndex]; + + let sampleInRange = false; + for (const { start, end } of ranges) { + if (sampleTime >= start && sampleTime <= end) { + sampleInRange = true; + break; + } } - } - if (!sampleInRange) { - newSamples.stack[sampleIndex] = null; + if (!sampleInRange) { + newTable.stack[tableIndex] = null; + } } + + return newTable; } const newThread = { ...thread, - samples: newSamples, + samples: filterTable(samples), }; + + if (jsAllocations) { + // Filter the JS allocations if there are any. + newThread.jsAllocations = filterTable(jsAllocations); + } + if (nativeAllocations) { + // Filter the native allocations if there are any. + newThread.nativeAllocations = filterTable(nativeAllocations); + } + return newThread; }); } From ad5f1503f43e066362ccae851e1f4f9dd684d85c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Mon, 15 May 2023 10:29:06 -0400 Subject: [PATCH 7/9] Fix smaller review nits --- src/components/shared/MarkerContextMenu.css | 3 ++- src/components/shared/MarkerContextMenu.js | 1 + src/profile-logic/transforms.js | 1 + src/test/store/transforms.test.js | 26 ++++++++++----------- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/components/shared/MarkerContextMenu.css b/src/components/shared/MarkerContextMenu.css index bfafc741b2..6164b60606 100644 --- a/src/components/shared/MarkerContextMenu.css +++ b/src/components/shared/MarkerContextMenu.css @@ -67,6 +67,7 @@ .react-contextmenu-item--selected .markerContextMenuIconCopyPayload, .react-contextmenu-item:hover .markerContextMenuIconCopyPayload, .react-contextmenu-item--selected .markerContextMenuSelectThread, -.react-contextmenu-item:hover .markerContextMenuSelectThread { +.react-contextmenu-item:hover .markerContextMenuSelectThread, +.react-contextmenu-item:hover .markerContextMenuIconFilterSamples { filter: invert(1); } diff --git a/src/components/shared/MarkerContextMenu.js b/src/components/shared/MarkerContextMenu.js index d781503de2..1037892859 100644 --- a/src/components/shared/MarkerContextMenu.js +++ b/src/components/shared/MarkerContextMenu.js @@ -515,6 +515,7 @@ class MarkerContextMenuImpl extends PureComponent { }} elems={{ strong: }} > + {/* Using a fragment here so we can have a strong tag inside. */} <> Drop samples outside of “{marker.name}” markers diff --git a/src/profile-logic/transforms.js b/src/profile-logic/transforms.js index 0a6511ce0c..d70a0ba5cf 100644 --- a/src/profile-logic/transforms.js +++ b/src/profile-logic/transforms.js @@ -543,6 +543,7 @@ export function applyTransformToCallNodePath( // 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); diff --git a/src/test/store/transforms.test.js b/src/test/store/transforms.test.js index eb08c7c085..cd0bc4a9fb 100644 --- a/src/test/store/transforms.test.js +++ b/src/test/store/transforms.test.js @@ -1836,7 +1836,7 @@ describe('"filter-samples" transform', function () { const threadIndex = 0; addMarkersToThreadWithCorrespondingSamples(profile.threads[threadIndex], [ [ - 'Marker A', + 'DOMEvent', 0, 0.5, { @@ -1846,17 +1846,17 @@ describe('"filter-samples" transform', function () { }, ], [ - 'Marker B', + 'Log', 0.5, 1.5, { - type: 'UserTiming', - name: 'measure-1', - entryType: 'measure', + type: 'Log', + name: 'Random log message', + module: 'RandomModule', }, ], [ - 'Marker C', + 'UserTiming', 1.5, 2.5, { @@ -1866,7 +1866,7 @@ describe('"filter-samples" transform', function () { }, ], [ - 'Marker C', + 'UserTiming', 2.5, 3.5, { @@ -1892,12 +1892,12 @@ describe('"filter-samples" transform', function () { ]); }); - it('filtered to range of "Marker A"', function () { + it('filtered to range of "DOMEvent"', function () { dispatch( addTransformToStack(threadIndex, { type: 'filter-samples', filterType: 'marker', - filter: 'Marker A', + filter: 'DOMEvent', }) ); const callTree = selectedThreadSelectors.getCallTree(getState()); @@ -1910,12 +1910,12 @@ describe('"filter-samples" transform', function () { dispatch(popTransformsFromStack(0)); }); - it('filtered to range of "Marker B"', function () { + it('filtered to range of "Log"', function () { dispatch( addTransformToStack(threadIndex, { type: 'filter-samples', filterType: 'marker', - filter: 'Marker B', + filter: 'Log', }) ); const callTree = selectedThreadSelectors.getCallTree(getState()); @@ -1929,12 +1929,12 @@ describe('"filter-samples" transform', function () { dispatch(popTransformsFromStack(0)); }); - it('filtered to multiple ranges of "Marker C"', function () { + it('filtered to multiple ranges of "UserTiming"', function () { dispatch( addTransformToStack(threadIndex, { type: 'filter-samples', filterType: 'marker', - filter: 'Marker C', + filter: 'UserTiming', }) ); const callTree = selectedThreadSelectors.getCallTree(getState()); From 99056135e390ef6300c5dde8a141125b0067c03b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Mon, 15 May 2023 10:50:25 -0400 Subject: [PATCH 8/9] Use string table index instead of the full string --- src/components/shared/MarkerContextMenu.js | 5 +++-- src/profile-logic/transforms.js | 15 ++++++++------- src/test/store/transforms.test.js | 17 ++++++++++++----- src/types/transforms.js | 3 ++- 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/components/shared/MarkerContextMenu.js b/src/components/shared/MarkerContextMenu.js index 1037892859..54f654fcfe 100644 --- a/src/components/shared/MarkerContextMenu.js +++ b/src/components/shared/MarkerContextMenu.js @@ -62,7 +62,7 @@ type StateProps = {| +markerIndex: MarkerIndex, +previewSelection: PreviewSelection, +committedRange: StartEndRange, - +thread: Thread | null, + +thread: Thread, +implementationFilter: ImplementationFilter, +getMarkerLabelToCopy: (MarkerIndex) => string, +profiledThreadIds: Set, @@ -192,12 +192,13 @@ class MarkerContextMenuImpl extends PureComponent { marker, addTransformToStack, changeSelectedTab, + thread, } = this.props; const { threadsKey } = rightClickedMarkerInfo; addTransformToStack(threadsKey, { type: 'filter-samples', filterType: 'marker', - filter: marker.name, + filter: thread.stringTable.indexForString(marker.name), }); // Then change the selected tab to call tree. diff --git a/src/profile-logic/transforms.js b/src/profile-logic/transforms.js index d70a0ba5cf..37724573ea 100644 --- a/src/profile-logic/transforms.js +++ b/src/profile-logic/transforms.js @@ -44,6 +44,7 @@ import type { FilterSamplesType, Marker, Milliseconds, + IndexIntoStringTable, } from 'firefox-profiler/types'; /** @@ -278,15 +279,14 @@ export function parseTransforms(transformString: string): TransformStack { } 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 [, shortFilterType, filter] = tuple; const filterType = convertToFullFilterType(shortFilterType); + const filterIndex = parseInt(filter, 10); transforms.push({ type: 'filter-samples', filterType, - filter: filterString, + filter: filterIndex, }); break; } @@ -419,7 +419,7 @@ export function getTransformLabelL10nIds( case 'marker': return { l10nId: 'TransformNavigator--drop-samples-outside-of-markers', - item: transform.filter, + item: stringTable.getString(transform.filter), }; default: throw assertExhaustiveCheck(transform.filterType); @@ -1774,14 +1774,15 @@ export function filterSamples( thread: Thread, markers: Marker[], filterType: FilterSamplesType, - filter: string + filter: IndexIntoStringTable ): Thread { return timeCode('filterSamples', () => { // Find the ranges to filter. + const filterString = thread.stringTable.getString(filter); let ranges: StartEndRange[]; switch (filterType) { case 'marker': - ranges = _findRangesByMarkerFilter(markers, filter); + ranges = _findRangesByMarkerFilter(markers, filterString); break; default: throw assertExhaustiveCheck(filterType); diff --git a/src/test/store/transforms.test.js b/src/test/store/transforms.test.js index cd0bc4a9fb..ea8e0d4245 100644 --- a/src/test/store/transforms.test.js +++ b/src/test/store/transforms.test.js @@ -1834,7 +1834,8 @@ describe('"filter-samples" transform', function () { D `); const threadIndex = 0; - addMarkersToThreadWithCorrespondingSamples(profile.threads[threadIndex], [ + const thread = profile.threads[threadIndex]; + addMarkersToThreadWithCorrespondingSamples(thread, [ [ 'DOMEvent', 0, @@ -1893,11 +1894,12 @@ describe('"filter-samples" transform', function () { }); it('filtered to range of "DOMEvent"', function () { + const DOMEventStringIndex = thread.stringTable.indexForString('DOMEvent'); dispatch( addTransformToStack(threadIndex, { type: 'filter-samples', filterType: 'marker', - filter: 'DOMEvent', + filter: DOMEventStringIndex, }) ); const callTree = selectedThreadSelectors.getCallTree(getState()); @@ -1911,11 +1913,12 @@ describe('"filter-samples" transform', function () { }); it('filtered to range of "Log"', function () { + const logStringIndex = thread.stringTable.indexForString('Log'); dispatch( addTransformToStack(threadIndex, { type: 'filter-samples', filterType: 'marker', - filter: 'Log', + filter: logStringIndex, }) ); const callTree = selectedThreadSelectors.getCallTree(getState()); @@ -1930,11 +1933,13 @@ describe('"filter-samples" transform', function () { }); it('filtered to multiple ranges of "UserTiming"', function () { + const userTimingStringIndex = + thread.stringTable.indexForString('UserTiming'); dispatch( addTransformToStack(threadIndex, { type: 'filter-samples', filterType: 'marker', - filter: 'UserTiming', + filter: userTimingStringIndex, }) ); const callTree = selectedThreadSelectors.getCallTree(getState()); @@ -1949,11 +1954,13 @@ describe('"filter-samples" transform', function () { }); it('filtered to a non-existent marker name', function () { + const nonExistentStringIndex = + thread.stringTable.indexForString('non-existent'); dispatch( addTransformToStack(threadIndex, { type: 'filter-samples', filterType: 'marker', - filter: 'non-existent', + filter: nonExistentStringIndex, }) ); const callTree = selectedThreadSelectors.getCallTree(getState()); diff --git a/src/types/transforms.js b/src/types/transforms.js index cdad5aeef3..7af5db6077 100644 --- a/src/types/transforms.js +++ b/src/types/transforms.js @@ -20,6 +20,7 @@ import type { IndexIntoFuncTable, IndexIntoResourceTable, IndexIntoCategoryList, + IndexIntoStringTable, } from './profile'; import type { CallNodePath, ThreadsKey } from './profile-derived'; import type { ImplementationFilter } from './actions'; @@ -342,7 +343,7 @@ export type TransformDefinitions = { +type: 'filter-samples', // Expand this type when you need to support more than just the marker. +filterType: FilterSamplesType, - +filter: string, + +filter: IndexIntoStringTable, |}, }; From 36a374b52b0ff47ceec9c531bb6ad9a3b39629d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Mon, 15 May 2023 11:23:25 -0400 Subject: [PATCH 9/9] Change the icon for the context menu item --- res/img/svg/filter.svg | 7 ------- src/components/shared/MarkerContextMenu.css | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) delete mode 100644 res/img/svg/filter.svg diff --git a/res/img/svg/filter.svg b/res/img/svg/filter.svg deleted file mode 100644 index 94f1f25c45..0000000000 --- a/res/img/svg/filter.svg +++ /dev/null @@ -1,7 +0,0 @@ - - - - - diff --git a/src/components/shared/MarkerContextMenu.css b/src/components/shared/MarkerContextMenu.css index 6164b60606..8c04b4b46e 100644 --- a/src/components/shared/MarkerContextMenu.css +++ b/src/components/shared/MarkerContextMenu.css @@ -52,7 +52,7 @@ } .markerContextMenuIconFilterSamples { - background-image: url(firefox-profiler-res/img/svg/filter.svg); + background-image: url(firefox-profiler-res/img/svg/drop-icon.svg); } /* These icons don't look nice when selected, unless if we invert them. However