Skip to content

Commit 6058ac8

Browse files
committed
gh-154086: Fix flamegraph thread sample counts
1 parent 8367448 commit 6058ac8

4 files changed

Lines changed: 83 additions & 31 deletions

File tree

Lib/profiling/sampling/_flamegraph_assets/flamegraph.js

Lines changed: 26 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,23 @@ function getDisplayName(moduleName, filename) {
9999
return filename;
100100
}
101101

102-
function selectFlamegraphData() {
103-
const baseData = isShowingElided ? elidedFlamegraphData : normalData;
102+
function selectFlamegraphData(selectedThreadId = null) {
103+
let baseData = isShowingElided ? elidedFlamegraphData : normalData;
104+
105+
if (selectedThreadId !== null) {
106+
baseData = filterDataByThread(baseData, selectedThreadId);
107+
}
104108

105109
if (!isInverted) {
106110
return baseData;
107111
}
108112

113+
// Thread-filtered trees have different values, so invert them after filtering
114+
// instead of using the cached all-thread tree.
115+
if (selectedThreadId !== null) {
116+
return generateInvertedFlamegraph(baseData);
117+
}
118+
109119
if (isShowingElided) {
110120
if (!invertedElidedData) {
111121
invertedElidedData = generateInvertedFlamegraph(baseData);
@@ -120,12 +130,11 @@ function selectFlamegraphData() {
120130
}
121131

122132
function updateFlamegraphView() {
123-
const selectedData = selectFlamegraphData();
124133
const selectedThreadId = currentThreadFilter !== 'all' ? parseInt(currentThreadFilter, 10) : null;
125-
const filteredData = selectedThreadId !== null ? filterDataByThread(selectedData, selectedThreadId) : selectedData;
126-
const tooltip = createPythonTooltip(filteredData);
127-
const chart = createFlamegraph(tooltip, filteredData.value, filteredData);
128-
renderFlamegraph(chart, filteredData);
134+
const selectedData = selectFlamegraphData(selectedThreadId);
135+
const tooltip = createPythonTooltip(selectedData);
136+
const chart = createFlamegraph(tooltip, selectedData.value, selectedData);
137+
renderFlamegraph(chart, selectedData);
129138
populateThreadStats(selectedData, selectedThreadId);
130139
}
131140

@@ -1209,7 +1218,7 @@ function initThreadFilter(data) {
12091218
const threadFilter = document.getElementById('thread-filter');
12101219
const threadSection = document.getElementById('thread-section');
12111220

1212-
if (!threadFilter || !data.threads) return;
1221+
if (!threadFilter || !data.threads || data.stats?.is_differential) return;
12131222

12141223
threadFilter.innerHTML = '<option value="all">All Threads</option>';
12151224

@@ -1238,11 +1247,17 @@ function filterByThread() {
12381247

12391248
function filterDataByThread(data, threadId) {
12401249
function filterNode(node) {
1241-
if (!node.threads || !node.threads.includes(threadId)) {
1250+
const threadValues = node.thread_values?.[threadId];
1251+
if (!threadValues) {
12421252
return null;
12431253
}
12441254

1245-
const filteredNode = { ...node, children: [] };
1255+
const filteredNode = {
1256+
...node,
1257+
value: threadValues[0],
1258+
self: threadValues[1],
1259+
children: []
1260+
};
12461261

12471262
if (node.children && Array.isArray(node.children)) {
12481263
filteredNode.children = node.children
@@ -1253,25 +1268,7 @@ function filterDataByThread(data, threadId) {
12531268
return filteredNode;
12541269
}
12551270

1256-
function recalculateValue(node) {
1257-
if (!node.children || node.children.length === 0) {
1258-
return node.value || 0;
1259-
}
1260-
const childrenValue = node.children.reduce((sum, child) => sum + recalculateValue(child), 0);
1261-
node.value = Math.max(node.value || 0, childrenValue);
1262-
return node.value;
1263-
}
1264-
1265-
const filteredRoot = { ...data, children: [] };
1266-
1267-
if (data.children && Array.isArray(data.children)) {
1268-
filteredRoot.children = data.children
1269-
.map(child => filterNode(child))
1270-
.filter(child => child !== null);
1271-
}
1272-
1273-
recalculateValue(filteredRoot);
1274-
return filteredRoot;
1271+
return filterNode(data);
12751272
}
12761273

12771274
// ============================================================================

Lib/profiling/sampling/stack_collector.py

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,13 @@ class FlamegraphCollector(StackTraceCollector):
7171
def __init__(self, *args, **kwargs):
7272
super().__init__(*args, **kwargs)
7373
self.stats = {}
74-
self._root = {"samples": 0, "children": {}, "threads": set()}
74+
self._root = {
75+
"samples": 0,
76+
"children": {},
77+
"threads": set(),
78+
"thread_samples": collections.Counter(),
79+
"thread_self": collections.Counter(),
80+
}
7581
self._total_samples = 0
7682
self._sample_count = 0 # Track actual number of samples (not thread traces)
7783
self._func_intern = {}
@@ -243,6 +249,15 @@ def convert_children(children, min_samples, path_info):
243249
"lineno": func[1],
244250
"funcname": funcname_idx,
245251
"threads": sorted(list(node.get("threads", set()))),
252+
"thread_values": {
253+
thread_id: [
254+
samples,
255+
node["thread_self"].get(thread_id, 0),
256+
]
257+
for thread_id, samples in sorted(
258+
node["thread_samples"].items()
259+
)
260+
},
246261
}
247262

248263
source = self._get_source_lines(func)
@@ -340,6 +355,12 @@ def convert_children(children, min_samples, path_info):
340355
"per_thread_stats": per_thread_stats_with_pct
341356
},
342357
"threads": sorted(list(self._all_threads)),
358+
"thread_values": {
359+
thread_id: [samples, 0]
360+
for thread_id, samples in sorted(
361+
self._root["thread_samples"].items()
362+
)
363+
},
343364
"strings": self._string_table.get_strings(),
344365
"opcode_mapping": opcode_mapping
345366
}
@@ -356,6 +377,7 @@ def process_frames(self, frames, thread_id, weight=1):
356377
"""
357378
# Reverse to root->leaf order for tree building
358379
self._root["samples"] += weight
380+
self._root["thread_samples"][thread_id] += weight
359381
self._total_samples += weight
360382
self._root["threads"].add(thread_id)
361383
self._all_threads.add(thread_id)
@@ -368,9 +390,18 @@ def process_frames(self, frames, thread_id, weight=1):
368390

369391
node = current["children"].get(func)
370392
if node is None:
371-
node = {"samples": 0, "children": {}, "threads": set(), "opcodes": collections.Counter(), "self": 0}
393+
node = {
394+
"samples": 0,
395+
"children": {},
396+
"threads": set(),
397+
"thread_samples": collections.Counter(),
398+
"thread_self": collections.Counter(),
399+
"opcodes": collections.Counter(),
400+
"self": 0,
401+
}
372402
current["children"][func] = node
373403
node["samples"] += weight
404+
node["thread_samples"][thread_id] += weight
374405
node["threads"].add(thread_id)
375406

376407
if opcode is not None:
@@ -380,6 +411,7 @@ def process_frames(self, frames, thread_id, weight=1):
380411

381412
if current is not self._root:
382413
current["self"] += weight
414+
current["thread_self"][thread_id] += weight
383415

384416
def _get_source_lines(self, func):
385417
filename, lineno, _ = func

Lib/test/test_profiling/test_sampling_profiler/test_collectors.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1336,6 +1336,27 @@ def test_flamegraph_collector_json_structure_includes_stats(self):
13361336
self.assertIn("gc_pct", thread_data)
13371337
self.assertIn("total", thread_data)
13381338

1339+
def test_flamegraph_nodes_include_per_thread_values(self):
1340+
collector = FlamegraphCollector(sample_interval_usec=1000)
1341+
root = MockFrameInfo("app.py", 1, "main")
1342+
collector.process_frames(
1343+
[MockFrameInfo("app.py", 10, "worker_a"), root],
1344+
thread_id=1,
1345+
weight=2,
1346+
)
1347+
collector.process_frames(
1348+
[MockFrameInfo("app.py", 20, "worker_b"), root],
1349+
thread_id=2,
1350+
weight=3,
1351+
)
1352+
1353+
data = collector._convert_to_flamegraph_format()
1354+
1355+
self.assertEqual(data["thread_values"], {1: [2, 0], 2: [3, 0]})
1356+
children_by_line = {child["lineno"]: child for child in data["children"]}
1357+
self.assertEqual(children_by_line[10]["thread_values"], {1: [2, 2]})
1358+
self.assertEqual(children_by_line[20]["thread_values"], {2: [3, 3]})
1359+
13391360
def test_flamegraph_collector_per_thread_gc_percentage(self):
13401361
"""Test that per-thread GC percentage uses total samples as denominator."""
13411362
collector = FlamegraphCollector(sample_interval_usec=1000)
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Store per-thread sample counts in Tachyon flamegraphs so filtering a thread
2+
updates frame widths and totals.

0 commit comments

Comments
 (0)