FlameGraph: Ensure total is only counted once for recursive function calls (#111548)

grafana-flamegraph: Ensure total is only counted once for recursive function calls

Example flamegraph: https://flamegraph.com/share/2bb59df3-9930-11f0-94ec-760777e76ccd
This commit is contained in:
Christian Simon 2025-09-25 13:31:06 +01:00 committed by GitHub
parent 89b988ca55
commit c5f6318b7b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 160 additions and 24 deletions

View File

@ -5,9 +5,10 @@ import { createDataFrame } from '@grafana/data';
import { FlameGraphDataContainer } from '../FlameGraph/dataTransform';
import { data } from '../FlameGraph/testData/dataNestedSet';
import { textToDataContainer } from '../FlameGraph/testHelpers';
import { ColorScheme } from '../types';
import FlameGraphTopTableContainer from './FlameGraphTopTableContainer';
import FlameGraphTopTableContainer, { buildFilteredTable } from './FlameGraphTopTableContainer';
describe('FlameGraphTopTableContainer', () => {
const setup = () => {
@ -52,7 +53,10 @@ describe('FlameGraphTopTableContainer', () => {
expect(cells).toHaveLength(60); // 16 rows
expect(cells[1].textContent).toEqual('net/http.HandlerFunc.ServeHTTP');
expect(cells[2].textContent).toEqual('31.7 K');
expect(cells[3].textContent).toEqual('31.7 Bil');
expect(cells[3].textContent).toEqual('5.58 Bil');
expect(cells[5].textContent).toEqual('total');
expect(cells[6].textContent).toEqual('16.5 K');
expect(cells[7].textContent).toEqual('16.5 Bil');
expect(cells[25].textContent).toEqual('net/http.(*conn).serve');
expect(cells[26].textContent).toEqual('5.63 K');
expect(cells[27].textContent).toEqual('5.63 Bil');
@ -83,3 +87,111 @@ describe('FlameGraphTopTableContainer', () => {
expect(mocks.onSandwich).toHaveBeenCalledWith('net/http.HandlerFunc.ServeHTTP');
});
});
describe('buildFilteredTable', () => {
it('should group data by label and sum values', () => {
const container = textToDataContainer(`
[0////]
[1][2]
[3][4]
`);
const result = buildFilteredTable(container!);
expect(result).toEqual({
'0': { self: 1, total: 7, totalRight: 0 },
'1': { self: 0, total: 3, totalRight: 0 },
'2': { self: 0, total: 3, totalRight: 0 },
'3': { self: 3, total: 3, totalRight: 0 },
'4': { self: 3, total: 3, totalRight: 0 },
});
});
it('should sum values for duplicate labels', () => {
const container = textToDataContainer(`
[0///]
[1][1]
`);
const result = buildFilteredTable(container!);
expect(result).toEqual({
'0': { self: 0, total: 6, totalRight: 0 },
'1': { self: 6, total: 6, totalRight: 0 },
});
});
it('should filter by matchedLabels when provided', () => {
const container = textToDataContainer(`
[0////]
[1][2]
[3][4]
`);
const matchedLabels = new Set(['1', '3']);
const result = buildFilteredTable(container!, matchedLabels);
expect(result).toEqual({
'1': { self: 0, total: 3, totalRight: 0 },
'3': { self: 3, total: 3, totalRight: 0 },
});
});
it('should handle empty matchedLabels set', () => {
const container = textToDataContainer(`
[0////]
[1][2]
[3][4]
`);
const matchedLabels = new Set<string>();
const result = buildFilteredTable(container!, matchedLabels);
expect(result).toEqual({});
});
it('should handle data with no matches', () => {
const container = textToDataContainer(`
[0////]
[1][2]
[3][4]
`);
const matchedLabels = new Set(['9']);
const result = buildFilteredTable(container!, matchedLabels);
expect(result).toEqual({});
});
it('should work without matchedLabels filter', () => {
const container = textToDataContainer(`
[0]
[1]
`);
const result = buildFilteredTable(container!);
expect(result).toEqual({
'0': { self: 0, total: 3, totalRight: 0 },
'1': { self: 3, total: 3, totalRight: 0 },
});
});
it('should not inflate totals for recursive calls', () => {
const container = textToDataContainer(`
[0////]
[1][2]
[3][4]
[0]
`);
const result = buildFilteredTable(container!);
expect(result).toEqual({
'0': { self: 4, total: 7, totalRight: 0 },
'1': { self: 0, total: 3, totalRight: 0 },
'2': { self: 0, total: 3, totalRight: 0 },
'3': { self: 0, total: 3, totalRight: 0 },
'4': { self: 3, total: 3, totalRight: 0 },
});
});
});

View File

@ -53,28 +53,7 @@ const FlameGraphTopTableContainer = memo(
onTableSort,
colorScheme,
}: Props) => {
const table = useMemo(() => {
// Group the data by label, we show only one row per label and sum the values
// TODO: should be by filename + funcName + linenumber?
let filteredTable: { [key: string]: TableData } = Object.create(null);
for (let i = 0; i < data.data.length; i++) {
const value = data.getValue(i);
const valueRight = data.getValueRight(i);
const self = data.getSelf(i);
const label = data.getLabel(i);
// If user is doing text search we filter out labels in the same way we highlight them in flame graph.
if (!matchedLabels || matchedLabels.has(label)) {
filteredTable[label] = filteredTable[label] || {};
filteredTable[label].self = filteredTable[label].self ? filteredTable[label].self + self : self;
filteredTable[label].total = filteredTable[label].total ? filteredTable[label].total + value : value;
filteredTable[label].totalRight = filteredTable[label].totalRight
? filteredTable[label].totalRight + valueRight
: valueRight;
}
}
return filteredTable;
}, [data, matchedLabels]);
const table = useMemo(() => buildFilteredTable(data, matchedLabels), [data, matchedLabels]);
const styles = useStyles2(getStyles);
const theme = useTheme2();
@ -124,6 +103,49 @@ const FlameGraphTopTableContainer = memo(
FlameGraphTopTableContainer.displayName = 'FlameGraphTopTableContainer';
function buildFilteredTable(data: FlameGraphDataContainer, matchedLabels?: Set<string>) {
// Group the data by label, we show only one row per label and sum the values
// TODO: should be by filename + funcName + linenumber?
let filteredTable: { [key: string]: TableData } = Object.create(null);
// Track call stack to detect recursive calls
const callStack: string[] = [];
for (let i = 0; i < data.data.length; i++) {
const value = data.getValue(i);
const valueRight = data.getValueRight(i);
const self = data.getSelf(i);
const label = data.getLabel(i);
const level = data.getLevel(i);
// Maintain call stack based on level changes
while (callStack.length > level) {
callStack.pop();
}
// Check if this is a recursive call (same label already in call stack)
const isRecursive = callStack.some((entry) => entry === label);
// If user is doing text search we filter out labels in the same way we highlight them in flame graph.
if (!matchedLabels || matchedLabels.has(label)) {
filteredTable[label] = filteredTable[label] || {};
filteredTable[label].self = filteredTable[label].self ? filteredTable[label].self + self : self;
// Only add to total if this is not a recursive call
if (!isRecursive) {
filteredTable[label].total = filteredTable[label].total ? filteredTable[label].total + value : value;
filteredTable[label].totalRight = filteredTable[label].totalRight
? filteredTable[label].totalRight + valueRight
: valueRight;
}
}
// Add current call to the stack
callStack.push(label);
}
return filteredTable;
}
function buildTableDataFrame(
data: FlameGraphDataContainer,
table: { [key: string]: TableData },
@ -365,4 +387,6 @@ const getStylesActionCell = () => {
};
};
export { buildFilteredTable };
export default FlameGraphTopTableContainer;