diff --git a/static/app/components/profiling/flamegraph/aggregateFlamegraphTreeTable.tsx b/static/app/components/profiling/flamegraph/aggregateFlamegraphTreeTable.tsx index 7911b62624e88e..344900cfed6f88 100644 --- a/static/app/components/profiling/flamegraph/aggregateFlamegraphTreeTable.tsx +++ b/static/app/components/profiling/flamegraph/aggregateFlamegraphTreeTable.tsx @@ -74,13 +74,17 @@ function makeSortFunction( a: VirtualizedTreeNode, b: VirtualizedTreeNode ) => { - return b.node.node.aggregate_duration_ns - a.node.node.aggregate_duration_ns; + const avgA = a.node.frame.averageCallDuration || 0; + const avgB = b.node.frame.averageCallDuration || 0; + return avgB - avgA; } : ( a: VirtualizedTreeNode, b: VirtualizedTreeNode ) => { - return a.node.node.aggregate_duration_ns - b.node.node.aggregate_duration_ns; + const avgA = a.node.frame.averageCallDuration || 0; + const avgB = b.node.frame.averageCallDuration || 0; + return avgA - avgB; }; } @@ -249,6 +253,14 @@ export function AggregateFlamegraphTreeTable({ abbreviation /> } + avgWeight={ + defined(r.item.node.frame.averageCallDuration) ? ( + + ) : undefined + } selfWeight={r.item.node.node.totalWeight.toFixed(0)} relativeSelfWeight={relativeWeight( referenceNode.node.totalWeight, @@ -424,9 +436,9 @@ export function AggregateFlamegraphTreeTable({ - {t('Duration')}{' '} + {t('Average Duration')}{' '} diff --git a/static/app/components/profiling/flamegraph/callTreeTable.tsx b/static/app/components/profiling/flamegraph/callTreeTable.tsx index 12157d0fd36a2d..71fd26a9a97a57 100644 --- a/static/app/components/profiling/flamegraph/callTreeTable.tsx +++ b/static/app/components/profiling/flamegraph/callTreeTable.tsx @@ -3,7 +3,9 @@ import styled from '@emotion/styled'; import {IconSettings} from 'sentry/icons/iconSettings'; import {IconUser} from 'sentry/icons/iconUser'; +import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; +import {defined} from 'sentry/utils'; import type {FlamegraphFrame} from 'sentry/utils/profiling/flamegraphFrame'; import type {VirtualizedTreeNode} from 'sentry/utils/profiling/hooks/useVirtualizedTree/VirtualizedTreeNode'; import type {VirtualizedTreeRenderedRow} from 'sentry/utils/profiling/hooks/useVirtualizedTree/virtualizedTreeUtils'; @@ -403,7 +405,7 @@ export function CallTreeTableRow({ref, ...props}: CallTreeTableRowProps) { ); } -interface CallTreeTableColumns { +interface BaseCallTreeTableColumns { formatDuration: (value: number) => string; frameColor: string; node: VirtualizedTreeNode; @@ -413,12 +415,17 @@ interface CallTreeTableColumns { opts?: {expandChildren: boolean} ) => void; referenceNode: FlamegraphFrame; + tabIndex: number; + type: 'count' | 'time'; +} + +interface CallTreeTableColumns extends BaseCallTreeTableColumns { relativeSelfWeight: number; relativeTotalWeight: number; selfWeight: number | React.ReactNode; - tabIndex: number; totalWeight: number | React.ReactNode; - type: 'count' | 'time'; + avgWeight?: React.ReactNode; + showAvg?: boolean; } export function CallTreeTableFixedColumns(props: CallTreeTableColumns) { @@ -437,34 +444,39 @@ export function CallTreeTableFixedColumns(props: CallTreeTableColumns) {
- {typeof props.totalWeight === 'number' - ? props.formatDuration(props.totalWeight) - : props.totalWeight} -
- {props.relativeTotalWeight.toFixed(1)}% -
-
-
- {props.node.node.node.frame.is_application ? ( - + {props.showAvg ? ( + defined(props.avgWeight) ? ( + props.avgWeight ) : ( - - )} -
+ {t('Unknown')} + ) + ) : ( + + {typeof props.totalWeight === 'number' + ? props.formatDuration(props.totalWeight) + : props.totalWeight} +
+ {props.relativeTotalWeight.toFixed(1)}% +
+
+
+ {props.node.node.node.frame.is_application ? ( + + ) : ( + + )} +
+ + )}
); } -export function CallTreeTableDynamicColumns( - props: Omit< - CallTreeTableColumns, - 'relativeTotalWeight' | 'relativeSelfWeight' | 'selfWeight' | 'totalWeight' - > -) { +export function CallTreeTableDynamicColumns(props: BaseCallTreeTableColumns) { const handleExpanding = (evt: React.MouseEvent) => { evt.stopPropagation(); props.onExpandClick(props.node, !props.node.expanded, { diff --git a/static/app/types/profiling.d.ts b/static/app/types/profiling.d.ts index 9560142c66f4d7..3b7f83ec7e7419 100644 --- a/static/app/types/profiling.d.ts +++ b/static/app/types/profiling.d.ts @@ -192,7 +192,7 @@ declare namespace Profiling { children?: Span[]; }; - type FrameInfo = { + type Frame = { col?: number; colno?: number; column?: number; @@ -219,8 +219,18 @@ declare namespace Profiling { // nodejs only columnNumber?: number; lineNumber?: number; + resourceId?: number; scriptName?: string; scriptId?: number; + + // metadata about the frame's occurrences and more + count?: number; + weight?: number; + }; + + type FrameInfo = { + count: number; + weight: number; }; type FunctionMetric = { @@ -313,7 +323,8 @@ declare namespace Profiling { > >; shared: { - frames: ReadonlyArray>; + frames: ReadonlyArray>; + frame_infos?: ReadonlyArray; profile_ids?: ReadonlyArray[]; profiles?: ReadonlyArray; }; diff --git a/static/app/utils/profiling/flamegraph.spec.tsx b/static/app/utils/profiling/flamegraph.spec.tsx index 7c290405f3f384..e3b5bb1c420810 100644 --- a/static/app/utils/profiling/flamegraph.spec.tsx +++ b/static/app/utils/profiling/flamegraph.spec.tsx @@ -271,12 +271,10 @@ describe('flamegraph', () => { ); expect(flamegraph.frames[1]!.frame.name).toBe('f0'); - expect(flamegraph.frames[1]!.frame.totalWeight).toBe(1); expect(flamegraph.frames[1]!.start).toBe(2); expect(flamegraph.frames[1]!.end).toBe(3); expect(flamegraph.frames[0]!.frame.name).toBe('f1'); - expect(flamegraph.frames[0]!.frame.totalWeight).toBe(2); expect(flamegraph.frames[0]!.start).toBe(0); expect(flamegraph.frames[0]!.end).toBe(2); }); diff --git a/static/app/utils/profiling/flamegraph.ts b/static/app/utils/profiling/flamegraph.ts index a9b18c1e818057..1262f3a4a1a8ed 100644 --- a/static/app/utils/profiling/flamegraph.ts +++ b/static/app/utils/profiling/flamegraph.ts @@ -145,7 +145,6 @@ export class Flamegraph { 0 ); this.root.end = this.root.start + weight; - this.root.frame.totalWeight += weight; let width = 0; diff --git a/static/app/utils/profiling/frame.tsx b/static/app/utils/profiling/frame.tsx index 973b718f80bb8a..2f71ab22852544 100644 --- a/static/app/utils/profiling/frame.tsx +++ b/static/app/utils/profiling/frame.tsx @@ -1,6 +1,7 @@ import {trimPackage} from 'sentry/components/events/interfaces/frame/utils'; import type {SymbolicatorStatus} from 'sentry/components/events/interfaces/types'; import {t} from 'sentry/locale'; +import {defined} from 'sentry/utils'; const ROOT_KEY = 'sentry root'; const BROWSER_EXTENSION_REGEXP = /^(\@moz-extension\:\/\/|chrome-extension\:\/\/)/; @@ -27,9 +28,9 @@ export class Frame { readonly isRoot: boolean; - totalWeight = 0; - selfWeight = 0; - aggregateDuration = 0; + readonly totalCallCount?: number; + readonly totalCallDuration?: number; + readonly averageCallDuration?: number; static Root = new Frame({ key: ROOT_KEY, @@ -38,40 +39,49 @@ export class Frame { }); constructor( - frameInfo: Profiling.FrameInfo, + frame: Profiling.Frame, type?: 'mobile' | 'javascript' | 'node' | string, // In aggregate mode, we miss certain info like lineno/col and so // we need to make sure we don't try to use it or infer data based on it mode?: 'detailed' | 'aggregate' ) { - this.key = frameInfo.key; - this.file = frameInfo.file; - this.name = frameInfo.name; - this.resource = frameInfo.resource; - this.line = frameInfo.line; - this.column = frameInfo.column; - this.is_application = !!frameInfo.is_application; - this.package = frameInfo.package; - this.module = frameInfo.module ?? frameInfo.image; - this.threadId = frameInfo.threadId; - this.path = frameInfo.path; - this.platform = frameInfo.platform; - this.instructionAddr = frameInfo.instructionAddr; - this.symbol = frameInfo.symbol; - this.symbolAddr = frameInfo.symbolAddr; - this.symbolicatorStatus = frameInfo.symbolicatorStatus; + this.key = frame.key; + this.file = frame.file; + this.name = frame.name; + this.resource = frame.resource; + this.line = frame.line; + this.column = frame.column; + this.is_application = !!frame.is_application; + this.package = frame.package; + this.module = frame.module ?? frame.image; + this.threadId = frame.threadId; + this.path = frame.path; + this.platform = frame.platform; + this.instructionAddr = frame.instructionAddr; + this.symbol = frame.symbol; + this.symbolAddr = frame.symbolAddr; + this.symbolicatorStatus = frame.symbolicatorStatus; this.isRoot = this.key === ROOT_KEY; + this.totalCallCount = frame.count; + this.totalCallDuration = frame.weight; + this.averageCallDuration = + defined(frame.weight) && defined(frame.count) + ? frame.count + ? frame.weight / frame.count + : 0 + : undefined; + // We are remapping some of the keys as they differ between platforms. // This is a temporary solution until we adopt a unified format. - if (frameInfo.columnNumber && this.column === undefined) { - this.column = frameInfo.columnNumber; + if (frame.columnNumber && this.column === undefined) { + this.column = frame.columnNumber; } - if (frameInfo.lineNumber && this.column === undefined) { - this.line = frameInfo.lineNumber; + if (frame.lineNumber && this.column === undefined) { + this.line = frame.lineNumber; } - if (frameInfo.scriptName && this.column === undefined) { - this.resource = frameInfo.scriptName; + if (frame.scriptName && this.column === undefined) { + this.resource = frame.scriptName; } // If the frame is a web frame and there is no name associated to it, then it was likely invoked as an iife or anonymous callback as diff --git a/static/app/utils/profiling/profile/continuousProfile.tsx b/static/app/utils/profiling/profile/continuousProfile.tsx index 4a261267277994..21a70ac507b9c2 100644 --- a/static/app/utils/profiling/profile/continuousProfile.tsx +++ b/static/app/utils/profiling/profile/continuousProfile.tsx @@ -138,10 +138,7 @@ export class ContinuousProfile extends Profile { child.lock(); } - node.frame.selfWeight += duration; - for (const stackNode of framesInStack) { - stackNode.frame.totalWeight += duration; stackNode.count++; } diff --git a/static/app/utils/profiling/profile/eventedProfile.tsx b/static/app/utils/profiling/profile/eventedProfile.tsx index f2305a186946ac..945a9e275868ab 100644 --- a/static/app/utils/profiling/profile/eventedProfile.tsx +++ b/static/app/utils/profiling/profile/eventedProfile.tsx @@ -97,19 +97,6 @@ export class EventedProfile extends Profile { return built; } - addWeightToFrames(weight: number): void { - const weightDelta = weight - this.lastValue; - - for (const frame of this.stack) { - frame.totalWeight += weightDelta; - } - - const top = this.stack[this.stack.length - 1]; - if (top) { - top.selfWeight += weight; - } - } - addWeightsToNodes(value: number) { const delta = value - this.lastValue; @@ -124,7 +111,6 @@ export class EventedProfile extends Profile { } enterFrame(frame: Frame, at: number): void { - this.addWeightToFrames(at); this.addWeightsToNodes(at); const lastTop = this.calltree[this.calltree.length - 1]; @@ -192,8 +178,8 @@ export class EventedProfile extends Profile { } leaveFrame(_event: Frame, at: number): void { - this.addWeightToFrames(at); this.addWeightsToNodes(at); + this.trackSampleStats(at); const leavingStackTop = this.calltree.pop(); diff --git a/static/app/utils/profiling/profile/importProfile.tsx b/static/app/utils/profiling/profile/importProfile.tsx index e065051f70eec2..3da8e8d9e64091 100644 --- a/static/app/utils/profiling/profile/importProfile.tsx +++ b/static/app/utils/profiling/profile/importProfile.tsx @@ -231,7 +231,14 @@ function importSchema( : input.metadata.platform === 'javascript' ? 'javascript' : 'mobile', - input.shared.frames + input.shared.frames.map((frame, i) => { + const frameInfo = input.shared.frame_infos?.[i]; + return { + ...frame, + count: frameInfo?.count, + weight: frameInfo?.weight, + }; + }) ); return { diff --git a/static/app/utils/profiling/profile/jsSelfProfile.tsx b/static/app/utils/profiling/profile/jsSelfProfile.tsx index 2670ad4d385815..e994e2ba256cdd 100644 --- a/static/app/utils/profiling/profile/jsSelfProfile.tsx +++ b/static/app/utils/profiling/profile/jsSelfProfile.tsx @@ -157,10 +157,7 @@ export class JSSelfProfile extends Profile { child.lock(); } - node.frame.selfWeight += weight; - for (const stackNode of framesInStack) { - stackNode.frame.totalWeight += weight; stackNode.count++; } diff --git a/static/app/utils/profiling/profile/sampledProfile.spec.tsx b/static/app/utils/profiling/profile/sampledProfile.spec.tsx index 71a089a7a76939..b5e17c63f052dc 100644 --- a/static/app/utils/profiling/profile/sampledProfile.spec.tsx +++ b/static/app/utils/profiling/profile/sampledProfile.spec.tsx @@ -229,14 +229,9 @@ describe('SampledProfile', () => { 'f1 [native code]' ); // The total weight of the previous top is now the weight of the GC call + the weight of the previous top - expect(profile.callTree.children[0]!.children[0]!.frame.totalWeight).toBe(4); expect(profile.callTree.children[0]!.children[0]!.children[0]!.frame.name).toBe( '(garbage collector) [native code]' ); - // The self weight of the GC call is only the weight of the GC call - expect(profile.callTree.children[0]!.children[0]!.children[0]!.frame.selfWeight).toBe( - 3 - ); }); it('places garbage collector calls on top of previous stack and skips stack', () => { @@ -333,9 +328,6 @@ describe('SampledProfile', () => { // There are no other children than the GC call meaning merge happened expect(profile.callTree.children[0]!.children[0]!.children[1]).toBeUndefined(); - expect(profile.callTree.children[0]!.children[0]!.children[0]!.frame.selfWeight).toBe( - 6 - ); }); it('flamegraph tracks node occurrences', () => { @@ -421,8 +413,5 @@ describe('SampledProfile', () => { expect(profile.callTree.children[0]!.aggregate_duration_ns).toBe(15); expect(profile.callTree.children[0]!.children[0]!.aggregate_duration_ns).toBe(10); expect(profile.callTree.children[0]!.children[1]!.aggregate_duration_ns).toBe(5); - expect(profile.callTree.children[0]!.frame.aggregateDuration).toBe(15); - expect(profile.callTree.children[0]!.children[0]!.frame.aggregateDuration).toBe(10); - expect(profile.callTree.children[0]!.children[1]!.frame.aggregateDuration).toBe(5); }); }); diff --git a/static/app/utils/profiling/profile/sampledProfile.tsx b/static/app/utils/profiling/profile/sampledProfile.tsx index 6fcab71d209b74..5135c57f1f40bf 100644 --- a/static/app/utils/profiling/profile/sampledProfile.tsx +++ b/static/app/utils/profiling/profile/sampledProfile.tsx @@ -327,11 +327,7 @@ export class SampledProfile extends Profile { child.lock(); } - node.frame.selfWeight += weight; - for (const stackNode of framesInStack) { - stackNode.frame.totalWeight += weight; - stackNode.frame.aggregateDuration += aggregate_duration_ns ?? 0; stackNode.count++; } diff --git a/static/app/utils/profiling/profile/sentrySampledProfile.tsx b/static/app/utils/profiling/profile/sentrySampledProfile.tsx index ba08c9e810243f..9d30f9e40b7614 100644 --- a/static/app/utils/profiling/profile/sentrySampledProfile.tsx +++ b/static/app/utils/profiling/profile/sentrySampledProfile.tsx @@ -138,10 +138,7 @@ export class SentrySampledProfile extends Profile { child.lock(); } - node.frame.selfWeight += weight; - for (const stackNode of framesInStack) { - stackNode.frame.totalWeight += weight; stackNode.count++; }