diff --git a/locales/en-US/app.ftl b/locales/en-US/app.ftl index 9475a20d4c..89e8893018 100644 --- a/locales/en-US/app.ftl +++ b/locales/en-US/app.ftl @@ -125,6 +125,15 @@ CallNodeContextMenu--transform-focus-category = Focus on category { $cat .title = Focusing on the nodes that belong to the same category as the selected node, thereby merging all nodes that belong to another category. + +# This is used as the context menu item to apply the "Drop category" transform. +# Variables: +# $categoryName (String) - Name of the category to drop. +CallNodeContextMenu--transform-drop-category = Drop samples with category { $categoryName } + .title = + Dropping samples with a category removes all samples whose leaf frame + belongs to that category. + CallNodeContextMenu--transform-collapse-function-subtree = Collapse function .title = Collapsing a function will remove everything it called, and assign @@ -1180,6 +1189,11 @@ TransformNavigator--focus-self = Focus Self: { $item } # $item (String) - Name of the category that transform applied to. TransformNavigator--focus-category = Focus category: { $item } +# "Drop samples with category" transform (drop is a verb, as in remove). +# Variables: +# $item (String) - Name of the category that transform applied to. +TransformNavigator--drop-category = Drop samples with category: { $item } + # "Merge call node" transform. # See: https://profiler.firefox.com/docs/#/./guide-filtering-call-trees?id=merge # Variables: diff --git a/src/actions/profile-view.ts b/src/actions/profile-view.ts index a7bf7ddc39..34bdfc7fab 100644 --- a/src/actions/profile-view.ts +++ b/src/actions/profile-view.ts @@ -2103,6 +2103,14 @@ export function handleCallNodeTransformShortcut( }) ); break; + case 'G': + dispatch( + addTransformToStack(threadsKey, { + type: 'drop-category', + category, + }) + ); + break; default: // This did not match a call tree transform. } diff --git a/src/app-logic/url-handling.ts b/src/app-logic/url-handling.ts index eecd06a2be..2430c5c2db 100644 --- a/src/app-logic/url-handling.ts +++ b/src/app-logic/url-handling.ts @@ -53,7 +53,7 @@ import { StringTable } from 'firefox-profiler/utils/string-table'; import type { ProfileUpgradeInfo } from 'firefox-profiler/profile-logic/processed-profile-versioning'; import type { ProfileAndProfileUpgradeInfo } from 'firefox-profiler/actions/receive-profile'; -export const CURRENT_URL_VERSION = 15; +export const CURRENT_URL_VERSION = 16; /** * This static piece of state might look like an anti-pattern, but it's a relatively @@ -1351,6 +1351,10 @@ const _upgraders: { .map(mapIndexesInTransform) .join('~'); }, + [16]: (_) => { + // Added the 'drop-category' transform (short key 'dg'). No existing URLs + // need rewriting; this is a new transform with no prior encoding to migrate. + }, }; for (let destVersion = 1; destVersion <= CURRENT_URL_VERSION; destVersion++) { diff --git a/src/components/shared/CallNodeContextMenu.tsx b/src/components/shared/CallNodeContextMenu.tsx index 1a360f01c6..561412df8b 100644 --- a/src/components/shared/CallNodeContextMenu.tsx +++ b/src/components/shared/CallNodeContextMenu.tsx @@ -422,6 +422,13 @@ class CallNodeContextMenuImpl extends React.PureComponent { }); break; } + case 'drop-category': { + addTransformToStack(threadsKey, { + type: 'drop-category', + category, + }); + break; + } case 'filter-samples': throw new Error( "Filter samples transform can't be applied from the call node context menu." @@ -714,6 +721,22 @@ class CallNodeContextMenuImpl extends React.PureComponent { }) : null} + {hasCategory + ? this.renderTransformMenuItem({ + l10nId: 'CallNodeContextMenu--transform-drop-category', + l10nProps: { + vars: { categoryName }, + elems: { strong: }, + }, + shortcut: 'G', + icon: 'Drop', + onClick: this._handleClick, + transform: 'drop-category', + title: '', + content: 'Drop samples with this category', + }) + : null} + {this.renderTransformMenuItem({ l10nId: 'CallNodeContextMenu--transform-collapse-function-subtree', shortcut: 'c', diff --git a/src/profile-logic/transforms.ts b/src/profile-logic/transforms.ts index 05ba86b2bc..783a5420b5 100644 --- a/src/profile-logic/transforms.ts +++ b/src/profile-logic/transforms.ts @@ -74,6 +74,7 @@ const TRANSFORM_OBJ: { [key in TransformType]: true } = { 'collapse-recursion': true, 'collapse-function-subtree': true, 'focus-category': true, + 'drop-category': true, 'filter-samples': true, }; export const ALL_TRANSFORM_TYPES: TransformType[] = Object.keys( @@ -100,6 +101,9 @@ ALL_TRANSFORM_TYPES.forEach((transform: TransformType) => { case 'focus-category': shortKey = 'fg'; break; + case 'drop-category': + shortKey = 'dg'; + break; case 'merge-call-node': shortKey = 'mcn'; break; @@ -277,6 +281,19 @@ export function parseTransforms(transformString: string): TransformStack { } break; } + case 'drop-category': { + // e.g. "dg-3" + const [, categoryRaw] = tuple; + const category = parseInt(categoryRaw, 10); + // Validate that the category makes sense. + if (!isNaN(category) && category >= 0) { + transforms.push({ + type: 'drop-category', + category, + }); + } + break; + } case 'focus-subtree': case 'merge-call-node': { // e.g. "f-js-xFFpUMl-i" or "f-cpp-0KV4KV5KV61KV7KV8K" @@ -378,6 +395,7 @@ export function stringifyTransforms(transformStack: TransformStack): string { case 'focus-function': return `${shortKey}-${transform.funcIndex}`; case 'focus-category': + case 'drop-category': return `${shortKey}-${transform.category}`; case 'collapse-resource': return `${shortKey}-${transform.implementation}-${transform.resourceIndex}-${transform.collapsedFuncIndex}`; @@ -449,6 +467,16 @@ export function getTransformLabelL10nIds( }; } + if (transform.type === 'drop-category') { + if (categories === undefined) { + throw new Error('Expected categories to be defined.'); + } + return { + l10nId: 'TransformNavigator--drop-category', + item: categories[transform.category].name, + }; + } + if (transform.type === 'filter-samples') { switch (transform.filterType) { case 'marker-search': @@ -551,6 +579,12 @@ export function applyTransformToCallNodePath( callNodePath, callNodeInfo ); + case 'drop-category': + return _dropCategoryInCallNodePath( + transform.category, + callNodePath, + callNodeInfo + ); case 'merge-call-node': return _mergeNodeInCallNodePath(transform.callNodePath, callNodePath); case 'merge-function': @@ -672,6 +706,23 @@ function _removeOtherCategoryFunctionsInNodePathWithFunction( return newCallNodePath; } +// If the leaf node of the call node path belongs to the dropped category, +// return an empty path — the whole sample is gone. +function _dropCategoryInCallNodePath( + category: IndexIntoCategoryList, + callNodePath: CallNodePath, + callNodeInfo: CallNodeInfo +): CallNodePath { + const leafCallNodeIndex = callNodeInfo.getCallNodeIndexFromPath(callNodePath); + if ( + leafCallNodeIndex !== null && + callNodeInfo.categoryForNode(leafCallNodeIndex) === category + ) { + return []; + } + return callNodePath; +} + function _collapseResourceInCallNodePath( resourceIndex: IndexIntoResourceTable, collapsedFuncIndex: IndexIntoFuncTable, @@ -1456,6 +1507,20 @@ export function focusCategory(thread: Thread, category: IndexIntoCategoryList) { }); } +/** + * Drop any samples whose leaf stack frame belongs to the given category. + */ +export function dropCategory(thread: Thread, category: IndexIntoCategoryList) { + return timeCode('dropCategory', () => { + const { stackTable } = thread; + + return updateThreadStacks(thread, stackTable, (stack) => + // Drop any sample whose leaf frame belongs to the given category. + stack !== null && stackTable.category[stack] === category ? null : stack + ); + }); +} + /** * When restoring function in a CallNodePath there can be multiple correct CallNodePaths * that could be restored. The best approach would probably be to restore to the @@ -1834,6 +1899,8 @@ export function applyTransform( return focusSelf(thread, transform.funcIndex, transform.implementation); case 'focus-category': return focusCategory(thread, transform.category); + case 'drop-category': + return dropCategory(thread, transform.category); case 'collapse-resource': return collapseResource( thread, @@ -2059,7 +2126,8 @@ export function translateTransform( funcIndex: newFuncIndex, }; } - case 'focus-category': { + case 'focus-category': + case 'drop-category': { // We don't sanitize-away categories, so this transform doesn't need to // be translated. return transform; diff --git a/src/test/components/CallNodeContextMenu.test.tsx b/src/test/components/CallNodeContextMenu.test.tsx index 0e032e73cf..85252b7e99 100644 --- a/src/test/components/CallNodeContextMenu.test.tsx +++ b/src/test/components/CallNodeContextMenu.test.tsx @@ -125,7 +125,8 @@ describe('calltree/CallNodeContextMenu', function () { { matcher: /Merge node only/, type: 'merge-call-node' }, { matcher: /Focus on subtree only/, type: 'focus-subtree' }, { matcher: /Focus on function/, type: 'focus-function' }, - { matcher: /Other/, type: 'focus-category' }, + { matcher: /Focus on category/, type: 'focus-category' }, + { matcher: /Drop samples with category/, type: 'drop-category' }, { matcher: /Collapse function/, type: 'collapse-function-subtree' }, { matcher: /XUL/, type: 'collapse-resource' }, { @@ -136,7 +137,7 @@ describe('calltree/CallNodeContextMenu', function () { matcher: /Collapse direct recursion/, type: 'collapse-direct-recursion', }, - { matcher: /Drop samples/, type: 'drop-function' }, + { matcher: /Drop samples with this function/, type: 'drop-function' }, ]; fixtures.forEach(({ matcher, type }) => { diff --git a/src/test/components/TransformShortcuts.test.tsx b/src/test/components/TransformShortcuts.test.tsx index aefbcfbd4a..17d5f6464c 100644 --- a/src/test/components/TransformShortcuts.test.tsx +++ b/src/test/components/TransformShortcuts.test.tsx @@ -68,6 +68,15 @@ function testTransformKeyboardShortcuts(setup: () => TestSetup) { }); }); + it('handles drop-category', () => { + const { pressKey, getTransform, expectedCategory } = setup(); + pressKey({ key: 'G' }); + expect(getTransform()).toEqual({ + type: 'drop-category', + category: expectedCategory, + }); + }); + it('handles merge call node', () => { const { pressKey, getTransform, expectedCallNodePath } = setup(); pressKey({ key: 'M' }); diff --git a/src/test/components/__snapshots__/CallNodeContextMenu.test.tsx.snap b/src/test/components/__snapshots__/CallNodeContextMenu.test.tsx.snap index 083afe2a7b..5e950fb1a8 100644 --- a/src/test/components/__snapshots__/CallNodeContextMenu.test.tsx.snap +++ b/src/test/components/__snapshots__/CallNodeContextMenu.test.tsx.snap @@ -137,6 +137,30 @@ exports[`calltree/CallNodeContextMenu basic rendering renders a full context men g +
(c.name === 'Graphics' ? i : -1)) + .filter((i) => i !== -1)[0]; + + return { + threadIndex, + categoryIndex, + funcNamesDict, + ...storeWithProfile(profile), + }; + } + + describe('drops samples where the leaf frame is in the category, keeps others', function () { + // Sample 1: C (leaf) is Graphics -> entire sample dropped + // Sample 2: E (leaf) is Layout -> kept + const { threadIndex, categoryIndex, getState, dispatch } = setup(` + A[cat:Layout] A[cat:Layout] + B[cat:Layout] D[cat:Layout] + C[cat:Graphics] E[cat:Layout] + `); + const originalCallTree = selectedThreadSelectors.getCallTree(getState()); + + it('starts as an unfiltered call tree', function () { + expect(formatTree(originalCallTree)).toEqual([ + '- A (total: 2, self: —)', + ' - B (total: 1, self: —)', + ' - C (total: 1, self: 1)', + ' - D (total: 1, self: —)', + ' - E (total: 1, self: 1)', + ]); + }); + + it('drops the sample with a Graphics leaf, keeps the one without', function () { + dispatch( + addTransformToStack(threadIndex, { + type: 'drop-category', + category: categoryIndex, + }) + ); + const callTree = selectedThreadSelectors.getCallTree(getState()); + expect(formatTree(callTree)).toEqual([ + '- A (total: 1, self: —)', + ' - D (total: 1, self: —)', + ' - E (total: 1, self: 1)', + ]); + }); + }); + + describe('does not drop a sample when only the root frame is in the category', function () { + const { threadIndex, categoryIndex, getState, dispatch } = setup(` + A[cat:Graphics] + B[cat:Layout] + `); + + it('keeps the sample since the leaf is not in the category', function () { + dispatch( + addTransformToStack(threadIndex, { + type: 'drop-category', + category: categoryIndex, + }) + ); + const callTree = selectedThreadSelectors.getCallTree(getState()); + expect(formatTree(callTree)).toEqual([ + '- A (total: 1, self: —)', + ' - B (total: 1, self: 1)', + ]); + }); + }); + + describe('drops a sample when a leaf frame is in the category', function () { + const { threadIndex, categoryIndex, getState, dispatch } = setup(` + A[cat:Layout] + B[cat:Layout] + C[cat:Graphics] + `); + + it('results in an empty call tree', function () { + dispatch( + addTransformToStack(threadIndex, { + type: 'drop-category', + category: categoryIndex, + }) + ); + const callTree = selectedThreadSelectors.getCallTree(getState()); + expect(formatTree(callTree)).toEqual([]); + }); + }); + + describe('selected call node path is cleared when the leaf is in the category', function () { + const { threadIndex, categoryIndex, getState, dispatch, funcNamesDict } = + setup(` + A[cat:Layout] A[cat:Layout] + B[cat:Layout] D[cat:Layout] + C[cat:Graphics] E[cat:Layout] + `); + + it('clears selected path when the leaf node is in the Graphics category', function () { + const { A, B, C } = funcNamesDict; + dispatch(changeSelectedCallNode(threadIndex, [A, B, C])); + dispatch( + addTransformToStack(threadIndex, { + type: 'drop-category', + category: categoryIndex, + }) + ); + const selectedCallNodePath = + selectedThreadSelectors.getSelectedCallNodePath(getState()); + expect(selectedCallNodePath).toEqual([]); + }); + }); +}); + describe('"collapse-resource" transform', function () { describe('combined implementation', function () { /** diff --git a/src/test/url-handling.test.ts b/src/test/url-handling.test.ts index 88ff994ee0..a046c24558 100644 --- a/src/test/url-handling.test.ts +++ b/src/test/url-handling.test.ts @@ -1308,7 +1308,7 @@ describe('url upgrading', function () { describe('URL serialization of the transform stack', function () { const transformString = - 'f-combined-0w2~mcn-combined-2w4~f-js-3w5-i~mf-6~ff-7~fg-42~cr-combined-8-9~' + + 'f-combined-0w2~mcn-combined-2w4~f-js-3w5-i~mf-6~ff-7~fg-42~dg-43~cr-combined-8-9~' + 'drec-combined-10~rec-11~df-12~cfs-13'; const { getState } = _getStoreWithURL({ search: '?transforms=' + transformString, @@ -1349,6 +1349,10 @@ describe('URL serialization of the transform stack', function () { type: 'focus-category', category: 42, }, + { + type: 'drop-category', + category: 43, + }, { type: 'collapse-resource', resourceIndex: 8, diff --git a/src/types/transforms.ts b/src/types/transforms.ts index 0837ced0d7..17b9d3fdfe 100644 --- a/src/types/transforms.ts +++ b/src/types/transforms.ts @@ -368,6 +368,32 @@ export type TransformDefinitions = { readonly category: IndexIntoCategoryList; }; + /** + * Drop any samples whose leaf stack frame belongs to the specified category. + * Only the leaf (innermost) frame is checked — if a non-leaf frame belongs to + * the category the sample is kept. An example with 'function:category' as the + * node name, dropping the Native category: + * + * A:JS A:JS + * / \ \ + * v v Drop Native v + * B:JS E:JS -> E:JS + * | | | + * v v v + * C:Native B:JS B:JS + * | | + * v v + * D:JS D:JS + * + * The left branch (B:JS → C:Native) is removed because the leaf C:Native + * belongs to the Native category. The right branch is kept in full because + * its leaf D:JS is not Native, even though no Native frames appear there. + */ + 'drop-category': { + readonly type: 'drop-category'; + readonly category: IndexIntoCategoryList; + }; + /** * Filter the samples in the thread by the filter. * Currently it only supports filtering by the marker name but can be extended diff --git a/src/utils/types.ts b/src/utils/types.ts index 1d6df05f5e..006ab88fdc 100644 --- a/src/utils/types.ts +++ b/src/utils/types.ts @@ -86,6 +86,7 @@ export function convertToTransformType(type: string): TransformType | null { case 'focus-subtree': case 'focus-function': case 'focus-category': + case 'drop-category': case 'focus-self': case 'collapse-resource': case 'collapse-direct-recursion':