Skip to content

Commit 02c9462

Browse files
committed
gh-154086: Make thread filtering exact
1 parent 6058ac8 commit 02c9462

3 files changed

Lines changed: 114 additions & 10 deletions

File tree

Lib/profiling/sampling/_flamegraph_assets/flamegraph.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -946,7 +946,9 @@ function formatDuration(seconds) {
946946

947947
function populateProfileSummary(data) {
948948
const stats = data.stats || {};
949-
const totalSamples = stats.total_samples || data.value || 0;
949+
const totalSamples = currentThreadFilter !== 'all'
950+
? (data.value ?? 0)
951+
: (stats.total_samples ?? data.value ?? 0);
950952
const duration = stats.duration_sec || 0;
951953
const sampleRate = stats.sample_rate || (duration > 0 ? totalSamples / duration : 0);
952954
const errorRate = stats.error_rate || 0;
@@ -1252,10 +1254,16 @@ function filterDataByThread(data, threadId) {
12521254
return null;
12531255
}
12541256

1257+
const {
1258+
thread_values: _threadValues,
1259+
thread_opcodes: threadOpcodes,
1260+
...sharedNode
1261+
} = node;
12551262
const filteredNode = {
1256-
...node,
1263+
...sharedNode,
12571264
value: threadValues[0],
12581265
self: threadValues[1],
1266+
opcodes: threadOpcodes?.[threadId] ?? {},
12591267
children: []
12601268
};
12611269

Lib/profiling/sampling/stack_collector.py

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,18 @@ def convert_children(children, min_samples, path_info):
226226
out = []
227227
for func, node in children.items():
228228
samples = node["samples"]
229-
if samples < min_samples:
229+
significant_for_thread = any(
230+
thread_samples >= max(
231+
1,
232+
int(
233+
self._root["thread_samples"][thread_id]
234+
* 0.001
235+
),
236+
)
237+
for thread_id, thread_samples
238+
in node["thread_samples"].items()
239+
)
240+
if samples < min_samples and not significant_for_thread:
230241
continue
231242

232243
# Intern all string components for maximum efficiency
@@ -270,6 +281,14 @@ def convert_children(children, min_samples, path_info):
270281
opcodes = node.get("opcodes", {})
271282
if opcodes:
272283
child_entry["opcodes"] = dict(opcodes)
284+
thread_opcodes = node.get("thread_opcodes")
285+
if thread_opcodes:
286+
child_entry["thread_opcodes"] = {
287+
thread_id: dict(counts)
288+
for thread_id, counts in sorted(
289+
thread_opcodes.items()
290+
)
291+
}
273292

274293
# Recurse
275294
child_entry["children"] = convert_children(
@@ -326,7 +345,25 @@ def convert_children(children, min_samples, path_info):
326345
opcode_mapping = get_opcode_mapping()
327346

328347
# If we only have one root child, make it the root to avoid redundant level
329-
if len(root_children) == 1:
348+
root_thread_values = {
349+
thread_id: [samples, 0]
350+
for thread_id, samples in sorted(
351+
self._root["thread_samples"].items()
352+
)
353+
}
354+
sole_root_covers_profile = (
355+
len(root_children) == 1
356+
and root_children[0]["value"] == total_samples
357+
and {
358+
thread_id: values[0]
359+
for thread_id, values
360+
in root_children[0]["thread_values"].items()
361+
} == {
362+
thread_id: values[0]
363+
for thread_id, values in root_thread_values.items()
364+
}
365+
)
366+
if sole_root_covers_profile:
330367
main_child = root_children[0]
331368
# Update name and label to indicate it's the program root
332369
old_name = self._string_table.get_string(main_child["name"])
@@ -355,12 +392,7 @@ def convert_children(children, min_samples, path_info):
355392
"per_thread_stats": per_thread_stats_with_pct
356393
},
357394
"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-
},
395+
"thread_values": root_thread_values,
364396
"strings": self._string_table.get_strings(),
365397
"opcode_mapping": opcode_mapping
366398
}
@@ -406,6 +438,10 @@ def process_frames(self, frames, thread_id, weight=1):
406438

407439
if opcode is not None:
408440
node["opcodes"][opcode] += weight
441+
thread_opcodes = node.setdefault("thread_opcodes", {})
442+
thread_opcodes.setdefault(
443+
thread_id, collections.Counter()
444+
)[opcode] += weight
409445

410446
current = node
411447

Lib/test/test_profiling/test_sampling_profiler/test_collectors.py

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1357,6 +1357,66 @@ def test_flamegraph_nodes_include_per_thread_values(self):
13571357
self.assertEqual(children_by_line[10]["thread_values"], {1: [2, 2]})
13581358
self.assertEqual(children_by_line[20]["thread_values"], {2: [3, 3]})
13591359

1360+
def test_flamegraph_pruning_preserves_low_volume_thread(self):
1361+
collector = FlamegraphCollector(sample_interval_usec=1000)
1362+
collector.process_frames(
1363+
[MockFrameInfo("app.py", 10, "busy")],
1364+
thread_id=1,
1365+
weight=1999,
1366+
)
1367+
collector.process_frames(
1368+
[MockFrameInfo("app.py", 20, "rare")],
1369+
thread_id=2,
1370+
)
1371+
1372+
data = collector._convert_to_flamegraph_format()
1373+
1374+
self.assertEqual(data["thread_values"], {1: [1999, 0], 2: [1, 0]})
1375+
children_by_line = {child["lineno"]: child for child in data["children"]}
1376+
self.assertEqual(children_by_line[10]["thread_values"], {1: [1999, 1999]})
1377+
self.assertEqual(children_by_line[20]["thread_values"], {2: [1, 1]})
1378+
1379+
def test_flamegraph_does_not_promote_incomplete_root(self):
1380+
collector = FlamegraphCollector(sample_interval_usec=1000)
1381+
collector.process_frames(
1382+
[MockFrameInfo("app.py", 1, "busy")],
1383+
thread_id=1,
1384+
weight=2000,
1385+
)
1386+
for line in range(2, 2002):
1387+
collector.process_frames(
1388+
[MockFrameInfo("app.py", line, f"fragment_{line}")],
1389+
thread_id=2,
1390+
)
1391+
1392+
data = collector._convert_to_flamegraph_format()
1393+
1394+
self.assertNotIn("filename", data)
1395+
self.assertEqual(data["thread_values"], {1: [2000, 0], 2: [2000, 0]})
1396+
self.assertEqual(len(data["children"]), 1)
1397+
self.assertEqual(data["children"][0]["thread_values"], {1: [2000, 2000]})
1398+
1399+
def test_flamegraph_nodes_include_per_thread_opcodes(self):
1400+
collector = FlamegraphCollector(sample_interval_usec=1000)
1401+
collector.process_frames(
1402+
[MockFrameInfo("app.py", 10, "worker", opcode=100)],
1403+
thread_id=1,
1404+
weight=2,
1405+
)
1406+
collector.process_frames(
1407+
[MockFrameInfo("app.py", 10, "worker", opcode=101)],
1408+
thread_id=2,
1409+
weight=3,
1410+
)
1411+
1412+
data = collector._convert_to_flamegraph_format()
1413+
1414+
self.assertEqual(data["opcodes"], {100: 2, 101: 3})
1415+
self.assertEqual(
1416+
data["thread_opcodes"],
1417+
{1: {100: 2}, 2: {101: 3}},
1418+
)
1419+
13601420
def test_flamegraph_collector_per_thread_gc_percentage(self):
13611421
"""Test that per-thread GC percentage uses total samples as denominator."""
13621422
collector = FlamegraphCollector(sample_interval_usec=1000)

0 commit comments

Comments
 (0)