Skip to content

Commit 7908641

Browse files
[3.15] gh-154088: Count elided stacks consistently (GH-154093) (#156386)
gh-154088: Count elided stacks consistently (GH-154093) (cherry picked from commit b22d175) Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>
1 parent 65cd7e1 commit 7908641

3 files changed

Lines changed: 86 additions & 5 deletions

File tree

Lib/profiling/sampling/stack_collector.py

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ def _get_module_name(self, filename, path_info):
212212
self._module_cache[filename] = module_name
213213
return module_name
214214

215-
def _convert_to_flamegraph_format(self):
215+
def _convert_to_flamegraph_format(self, *, min_samples=None):
216216
if self._total_samples == 0:
217217
return {
218218
"name": self._string_table.intern("No Data"),
@@ -302,7 +302,8 @@ def convert_children(children, min_samples, path_info):
302302

303303
# Filter out very small functions (less than 0.1% of total samples)
304304
total_samples = self._total_samples
305-
min_samples = max(1, int(total_samples * 0.001))
305+
if min_samples is None:
306+
min_samples = max(1, int(total_samples * 0.001))
306307
path_info = get_python_path_info()
307308

308309
root_children = convert_children(self._root["children"], min_samples, path_info)
@@ -690,7 +691,15 @@ def _add_elided_flamegraph(self, current_flamegraph, current_stats, baseline_sta
690691
"""Calculate elided paths and add elided flamegraph to stats."""
691692
self._elided_paths = baseline_stats.keys() - current_stats.keys()
692693

693-
current_flamegraph["stats"]["elided_count"] = len(self._elided_paths)
694+
# A sampled stack can end at an internal path that also has elided
695+
# descendants. Count every disappeared path with self samples, not
696+
# just the leaves of the elided path tree.
697+
elided_stacks = {
698+
path
699+
for path in self._elided_paths
700+
if baseline_stats[path]["self"] > 0
701+
}
702+
current_flamegraph["stats"]["elided_count"] = len(elided_stacks)
694703

695704
if self._elided_paths:
696705
elided_flamegraph = self._build_elided_flamegraph(baseline_stats, scale)
@@ -713,7 +722,9 @@ def _build_elided_flamegraph(self, baseline_stats, scale):
713722
orig_get_source = self._baseline_collector._get_source_lines
714723
self._baseline_collector._get_source_lines = lambda func: None
715724
try:
716-
baseline_data = self._baseline_collector._convert_to_flamegraph_format()
725+
baseline_data = self._baseline_collector._convert_to_flamegraph_format(
726+
min_samples=1
727+
)
717728
finally:
718729
self._baseline_collector._get_source_lines = orig_get_source
719730

Lib/test/test_profiling/test_sampling_profiler/test_collectors.py

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1665,7 +1665,7 @@ def test_diff_flamegraph_elided_stacks(self):
16651665

16661666
data = diff._convert_to_flamegraph_format()
16671667

1668-
self.assertGreater(data["stats"]["elided_count"], 0)
1668+
self.assertEqual(data["stats"]["elided_count"], 1)
16691669
self.assertIn("elided_flamegraph", data["stats"])
16701670
elided = data["stats"]["elided_flamegraph"]
16711671
self.assertTrue(elided["stats"]["is_differential"])
@@ -1681,6 +1681,74 @@ def test_diff_flamegraph_elided_stacks(self):
16811681
self.assertGreater(child["baseline"], 0)
16821682
self.assertAlmostEqual(child["diff"], -child["baseline"])
16831683

1684+
def test_diff_flamegraph_counts_elided_stacks_not_paths(self):
1685+
"""Internal and leaf stack endings are counted separately."""
1686+
internal_stack = [
1687+
MockInterpreterInfo(0, [
1688+
MockThreadInfo(1, [
1689+
MockFrameInfo("file.py", 20, "old_mid"),
1690+
MockFrameInfo("file.py", 10, "root"),
1691+
])
1692+
])
1693+
]
1694+
leaf_stack = [
1695+
MockInterpreterInfo(0, [
1696+
MockThreadInfo(1, [
1697+
MockFrameInfo("file.py", 30, "old_leaf"),
1698+
MockFrameInfo("file.py", 20, "old_mid"),
1699+
MockFrameInfo("file.py", 10, "root"),
1700+
])
1701+
])
1702+
]
1703+
current_frames = [
1704+
MockInterpreterInfo(0, [
1705+
MockThreadInfo(1, [MockFrameInfo("file.py", 10, "root")])
1706+
])
1707+
]
1708+
1709+
diff = make_diff_collector_with_mock_baseline(
1710+
[internal_stack, leaf_stack]
1711+
)
1712+
diff.collect(current_frames)
1713+
1714+
data = diff._convert_to_flamegraph_format()
1715+
self.assertEqual(data["stats"]["elided_count"], 2)
1716+
1717+
def test_diff_flamegraph_renders_small_elided_stack(self):
1718+
"""Elided stacks are not removed by the significance filter."""
1719+
common_frames = [
1720+
MockInterpreterInfo(0, [
1721+
MockThreadInfo(1, [
1722+
MockFrameInfo("file.py", 20, "common"),
1723+
MockFrameInfo("file.py", 10, "root"),
1724+
])
1725+
])
1726+
]
1727+
old_frames = [
1728+
MockInterpreterInfo(0, [
1729+
MockThreadInfo(1, [
1730+
MockFrameInfo("file.py", 30, "old_tiny"),
1731+
MockFrameInfo("file.py", 10, "root"),
1732+
])
1733+
])
1734+
]
1735+
1736+
diff = make_diff_collector_with_mock_baseline(
1737+
[common_frames] * 1999 + [old_frames]
1738+
)
1739+
for _ in range(1999):
1740+
diff.collect(common_frames)
1741+
1742+
data = diff._convert_to_flamegraph_format()
1743+
self.assertEqual(data["stats"]["elided_count"], 1)
1744+
self.assertIn("elided_flamegraph", data["stats"])
1745+
1746+
elided = data["stats"]["elided_flamegraph"]
1747+
strings = elided["strings"]
1748+
self.assertIsNotNone(
1749+
find_child_by_name(elided.get("children", []), strings, "old_tiny")
1750+
)
1751+
16841752
def test_diff_flamegraph_elided_top_level_root(self):
16851753
"""Elided top-level roots do not crash metadata generation."""
16861754
baseline_frames_1 = [
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Make Tachyon count and render elided stacks consistently in differential
2+
flamegraphs.

0 commit comments

Comments
 (0)