From a0f2f40ff9960dc594a134d5f0d56d6f447c171e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A1szl=C3=B3=20Kiss=20Koll=C3=A1r?= Date: Tue, 5 May 2026 21:34:08 +0100 Subject: [PATCH 1/3] Fix heatmap line highlight replay The line highlights on the heatmap are driven by the URL hash and the `:target` selector. When clicking a caller/callee link for the line that was already selected, the hash doesn't change, so the browser keeps the existing target state and doesn't restart the animation. Due to this the highlight only works the first time. With this fix, line navigation goes through JavaScript. If the target URL already points to the current location, the highlight is replayed by clearing the animation, forcing style recalculation, and restoring it. --- .../sampling/_heatmap_assets/heatmap.js | 22 +++++++++++++++++-- .../test_sampling_profiler/test_collectors.py | 13 +++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/Lib/profiling/sampling/_heatmap_assets/heatmap.js b/Lib/profiling/sampling/_heatmap_assets/heatmap.js index 2da1103b82a52a..1f698779f3a46e 100644 --- a/Lib/profiling/sampling/_heatmap_assets/heatmap.js +++ b/Lib/profiling/sampling/_heatmap_assets/heatmap.js @@ -84,7 +84,7 @@ function showNavigationMenu(button, items, title) { item.appendChild(funcDiv); item.appendChild(createElement('div', 'callee-menu-file', linkData.file)); - item.addEventListener('click', () => window.location.href = linkData.link); + item.addEventListener('click', () => navigateToLine(linkData.link)); menu.appendChild(item); }); @@ -105,7 +105,7 @@ function handleNavigationClick(button, e) { const navData = button.getAttribute('data-nav'); if (navData) { - window.location.href = JSON.parse(navData).link; + navigateToLine(JSON.parse(navData).link); return; } @@ -117,11 +117,29 @@ function handleNavigationClick(button, e) { } } +function restartLineHighlight(target) { + target.style.animation = 'none'; + // Force style recalculation so restoring the animation restarts it. + void target.offsetWidth; + target.style.animation = ''; +} + +function navigateToLine(link) { + const url = new URL(link, window.location.href); + + if (url.href === window.location.href) { + scrollToTargetLine(); + } else { + window.location.href = link; + } +} + function scrollToTargetLine() { if (!window.location.hash) return; const target = document.querySelector(window.location.hash); if (target) { target.scrollIntoView({ behavior: 'smooth', block: 'start' }); + restartLineHighlight(target); } } diff --git a/Lib/test/test_profiling/test_sampling_profiler/test_collectors.py b/Lib/test/test_profiling/test_sampling_profiler/test_collectors.py index 240ec8a195c43b..bbde31ed6b964f 100644 --- a/Lib/test/test_profiling/test_sampling_profiler/test_collectors.py +++ b/Lib/test/test_profiling/test_sampling_profiler/test_collectors.py @@ -17,6 +17,7 @@ FlamegraphCollector, ) from profiling.sampling.gecko_collector import GeckoCollector + from profiling.sampling.heatmap_collector import _TemplateLoader from profiling.sampling.collector import extract_lineno, normalize_location from profiling.sampling.opcode_utils import get_opcode_info, format_opcode from profiling.sampling.constants import ( @@ -81,6 +82,18 @@ def test_mock_frame_info_with_empty_and_unicode_values(self): self.assertEqual(frame.location.lineno, 999999) self.assertEqual(frame.funcname, long_funcname) + def test_heatmap_navigation_restarts_line_highlight(self): + """Test heatmap navigation can replay target line highlights.""" + loader = _TemplateLoader() + + self.assertIn(".code-line:target", loader.file_css) + self.assertIn("function restartLineHighlight(target)", loader.file_js) + self.assertIn("target.style.animation = 'none'", loader.file_js) + self.assertIn("void target.offsetWidth", loader.file_js) + self.assertIn("url.href === window.location.href", loader.file_js) + self.assertIn("navigateToLine(JSON.parse(navData).link)", loader.file_js) + self.assertIn("navigateToLine(linkData.link)", loader.file_js) + def test_pstats_collector_with_extreme_intervals_and_empty_data(self): """Test PstatsCollector handles zero/large intervals, empty frames, None thread IDs, and duplicate frames.""" # Test with zero interval From 15d54fe489839f059ebb1c23f6a1571b6c9cc9b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A1szl=C3=B3=20Kiss=20Koll=C3=A1r?= Date: Mon, 4 May 2026 15:22:19 +0100 Subject: [PATCH 2/3] Fix diff flamegraph elided root crash The `baseline_self` variable isn't initialized for structural elided roots. This variable is accessed later unconditionally and leads to a crash. --- Lib/profiling/sampling/stack_collector.py | 2 ++ .../test_sampling_profiler/test_collectors.py | 33 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/Lib/profiling/sampling/stack_collector.py b/Lib/profiling/sampling/stack_collector.py index 04622a8c1e89ef..60df026ed76a6c 100644 --- a/Lib/profiling/sampling/stack_collector.py +++ b/Lib/profiling/sampling/stack_collector.py @@ -698,6 +698,8 @@ def _add_elided_metadata(self, node, baseline_stats, scale, path): func_key = self._extract_func_key(node, self._baseline_collector._string_table) current_path = path + (func_key,) if func_key else path + baseline_self = 0 + baseline_total = 0 if func_key and current_path in baseline_stats: baseline_data = baseline_stats[current_path] baseline_self = baseline_data["self"] * scale diff --git a/Lib/test/test_profiling/test_sampling_profiler/test_collectors.py b/Lib/test/test_profiling/test_sampling_profiler/test_collectors.py index bbde31ed6b964f..fc3b01a919e59c 100644 --- a/Lib/test/test_profiling/test_sampling_profiler/test_collectors.py +++ b/Lib/test/test_profiling/test_sampling_profiler/test_collectors.py @@ -1415,6 +1415,39 @@ def test_diff_flamegraph_elided_stacks(self): self.assertGreater(child["baseline"], 0) self.assertAlmostEqual(child["diff"], -child["baseline"]) + def test_diff_flamegraph_elided_top_level_root(self): + """Elided top-level roots do not crash metadata generation.""" + baseline_frames_1 = [ + MockInterpreterInfo(0, [ + MockThreadInfo(1, [ + MockFrameInfo("file.py", 10, "kept_leaf"), + MockFrameInfo("file.py", 20, "kept_root"), + ]) + ]) + ] + baseline_frames_2 = [ + MockInterpreterInfo(0, [ + MockThreadInfo(1, [ + MockFrameInfo("file.py", 30, "old_leaf"), + MockFrameInfo("file.py", 40, "old_root"), + ]) + ]) + ] + + diff = make_diff_collector_with_mock_baseline([ + baseline_frames_1, + baseline_frames_2, + ]) + diff.collect(baseline_frames_1) + + data = diff._convert_to_flamegraph_format() + elided = data["stats"]["elided_flamegraph"] + elided_strings = elided.get("strings", []) + children = elided.get("children", []) + + self.assertEqual(len(children), 1) + self.assertIn("old_root", resolve_name(children[0], elided_strings)) + def test_diff_flamegraph_function_matched_despite_line_change(self): """Functions match by (filename, funcname), ignoring lineno.""" baseline_frames = [ From 7a11925724f5cdee8282bb85779f947005d6e79e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A1szl=C3=B3=20Kiss=20Koll=C3=A1r?= Date: Mon, 4 May 2026 19:05:11 +0100 Subject: [PATCH 3/3] Fix child diff flamegraph args The child process ends up being invoked with `--diff_flamegraph` instead of the correct argument. --- Lib/profiling/sampling/cli.py | 4 ++- .../test_sampling_profiler/test_children.py | 33 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/Lib/profiling/sampling/cli.py b/Lib/profiling/sampling/cli.py index c0aa3ae024a120..b57eef94699d49 100644 --- a/Lib/profiling/sampling/cli.py +++ b/Lib/profiling/sampling/cli.py @@ -160,7 +160,9 @@ def _build_child_profiler_args(args): child_args.extend(["--mode", mode]) # Format options (skip pstats as it's the default) - if args.format != "pstats": + if args.format == "diff_flamegraph": + child_args.extend(["--diff-flamegraph", args.diff_baseline]) + elif args.format != "pstats": child_args.append(f"--{args.format}") return child_args diff --git a/Lib/test/test_profiling/test_sampling_profiler/test_children.py b/Lib/test/test_profiling/test_sampling_profiler/test_children.py index bb49faa890f348..e64d917eedde56 100644 --- a/Lib/test/test_profiling/test_sampling_profiler/test_children.py +++ b/Lib/test/test_profiling/test_sampling_profiler/test_children.py @@ -109,6 +109,39 @@ def _wait_for_process_ready(proc, timeout): return proc.poll() is None +@unittest.skipIf( + _build_child_profiler_args is None, + "profiling.sampling.cli unavailable", +) +class TestChildProfilerArgBuilder(unittest.TestCase): + """Tests for child profiler CLI argument construction.""" + + def test_build_child_profiler_args_diff_flamegraph(self): + """Test child args use the real --diff-flamegraph flag.""" + args = argparse.Namespace( + sample_interval_usec=1000, + duration=None, + all_threads=False, + realtime_stats=False, + native=False, + gc=True, + opcodes=False, + async_aware=False, + mode="wall", + format="diff_flamegraph", + diff_baseline="baseline.bin", + ) + + child_args = _build_child_profiler_args(args) + + self.assertIn("--diff-flamegraph", child_args) + self.assertNotIn("--diff_flamegraph", child_args) + + flag_index = child_args.index("--diff-flamegraph") + self.assertGreater(len(child_args), flag_index + 1) + self.assertEqual(child_args[flag_index + 1], "baseline.bin") + + @requires_remote_subprocess_debugging() class TestGetChildPids(unittest.TestCase): """Tests for the get_child_pids function."""