Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,17 @@ function makeSortFunction(
a: VirtualizedTreeNode<FlamegraphFrame>,
b: VirtualizedTreeNode<FlamegraphFrame>
) => {
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<FlamegraphFrame>,
b: VirtualizedTreeNode<FlamegraphFrame>
) => {
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;
};
}

Expand Down Expand Up @@ -249,6 +253,14 @@ export function AggregateFlamegraphTreeTable({
abbreviation
/>
}
avgWeight={
defined(r.item.node.frame.averageCallDuration) ? (
<PerformanceDuration
nanoseconds={r.item.node.frame.averageCallDuration}
abbreviation
/>
) : undefined
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Missing Prop Causes Column Header Mismatch

The CallTreeTableFixedColumns component isn't displaying the average duration. While avgWeight is passed, the showAvg prop is missing, causing the component to default to showing totalWeight. This creates a mismatch with the "Average Duration" column header.

Fix in Cursor Fix in Web

selfWeight={r.item.node.node.totalWeight.toFixed(0)}
relativeSelfWeight={relativeWeight(
referenceNode.node.totalWeight,
Expand Down Expand Up @@ -424,9 +436,9 @@ export function AggregateFlamegraphTreeTable({
<CallTreeTableHeaderButton onClick={onSortByDuration}>
<InteractionStateLayer />
<span>
{t('Duration')}{' '}
{t('Average Duration')}{' '}
<QuestionTooltip
title={t('Aggregated duration of this frame across different samples.')}
title={t('Average duration of this frame across different samples.')}
size="sm"
position="top"
/>
Expand Down
62 changes: 37 additions & 25 deletions static/app/components/profiling/flamegraph/callTreeTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -403,7 +405,7 @@ export function CallTreeTableRow({ref, ...props}: CallTreeTableRowProps) {
);
}

interface CallTreeTableColumns {
interface BaseCallTreeTableColumns {
formatDuration: (value: number) => string;
frameColor: string;
node: VirtualizedTreeNode<FlamegraphFrame>;
Expand All @@ -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) {
Expand All @@ -437,34 +444,39 @@ export function CallTreeTableFixedColumns(props: CallTreeTableColumns) {
</div>
</div>
<div className={CallTreeTableClassNames.CELL} style={TEXT_ALIGN_RIGHT}>
{typeof props.totalWeight === 'number'
? props.formatDuration(props.totalWeight)
: props.totalWeight}
<div className={CallTreeTableClassNames.WEIGHT}>
{props.relativeTotalWeight.toFixed(1)}%
<div
className={CallTreeTableClassNames.BACKGROUND_WEIGHT}
style={{transform: `scaleX(${props.relativeTotalWeight / 100})`}}
/>
</div>
<div className={CallTreeTableClassNames.FRAME_TYPE}>
{props.node.node.node.frame.is_application ? (
<IconUser size="xs" />
{props.showAvg ? (
defined(props.avgWeight) ? (
props.avgWeight
) : (
<IconSettings size="xs" />
)}
</div>
<span>{t('Unknown')}</span>
)
) : (
<Fragment>
{typeof props.totalWeight === 'number'
? props.formatDuration(props.totalWeight)
: props.totalWeight}
<div className={CallTreeTableClassNames.WEIGHT}>
{props.relativeTotalWeight.toFixed(1)}%
<div
className={CallTreeTableClassNames.BACKGROUND_WEIGHT}
style={{transform: `scaleX(${props.relativeTotalWeight / 100})`}}
/>
</div>
<div className={CallTreeTableClassNames.FRAME_TYPE}>
{props.node.node.node.frame.is_application ? (
<IconUser size="xs" />
) : (
<IconSettings size="xs" />
)}
</div>
</Fragment>
)}
</div>
</Fragment>
);
}

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, {
Expand Down
15 changes: 13 additions & 2 deletions static/app/types/profiling.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ declare namespace Profiling {
children?: Span[];
};

type FrameInfo = {
type Frame = {
col?: number;
colno?: number;
column?: number;
Expand All @@ -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 = {
Expand Down Expand Up @@ -313,7 +323,8 @@ declare namespace Profiling {
>
>;
shared: {
frames: ReadonlyArray<Omit<Profiling.FrameInfo, 'key'>>;
frames: ReadonlyArray<Omit<Profiling.Frame, 'key'>>;
frame_infos?: ReadonlyArray<Profiling.FrameInfo, 'key'>;
profile_ids?: ReadonlyArray<string>[];
profiles?: ReadonlyArray<ProfileReference>;
};
Expand Down
2 changes: 0 additions & 2 deletions static/app/utils/profiling/flamegraph.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
1 change: 0 additions & 1 deletion static/app/utils/profiling/flamegraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ export class Flamegraph {
0
);
this.root.end = this.root.start + weight;
this.root.frame.totalWeight += weight;

let width = 0;

Expand Down
62 changes: 36 additions & 26 deletions static/app/utils/profiling/frame.tsx
Original file line number Diff line number Diff line change
@@ -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\:\/\/)/;
Expand All @@ -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,
Expand All @@ -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
Expand Down
3 changes: 0 additions & 3 deletions static/app/utils/profiling/profile/continuousProfile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
}

Expand Down
16 changes: 1 addition & 15 deletions static/app/utils/profiling/profile/eventedProfile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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];
Expand Down Expand Up @@ -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();
Expand Down
9 changes: 8 additions & 1 deletion static/app/utils/profiling/profile/importProfile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 0 additions & 3 deletions static/app/utils/profiling/profile/jsSelfProfile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
}

Expand Down
Loading
Loading