gh-149430: Fix edge-cases in `profiling.sampling` outputs (#149431)

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.

The `baseline_self` variable isn't initialized for structural elided
roots. This variable is accessed later unconditionally and leads to a
crash.

The child process ends up being invoked with `--diff_flamegraph` instead
of the correct argument.

diff --git a/Lib/profiling/sampling/_heatmap_assets/heatmap.js b/Lib/profiling/sampling/_heatmap_assets/heatmap.js
index 2da1103b82a52a34e648ae9e6db97ea415dcd5e7..1f698779f3a46e3eea7d6493bf249064c46ba3ca 100644 (file)
--- 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/profiling/sampling/cli.py b/Lib/profiling/sampling/cli.py
index 0648713edc52af3d44e0ca1f571baee2f280dd64..a5d9573ae6b6dddc0f8dddf3217c9613f6bec759 100644 (file)
--- a/Lib/profiling/sampling/cli.py
+++ b/Lib/profiling/sampling/cli.py
@@ -167,7 +167,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/profiling/sampling/stack_collector.py b/Lib/profiling/sampling/stack_collector.py
index 04622a8c1e89ef628118c5d8ae17443d158c7c6b..60df026ed76a6cec73fc036dec9272f75882191c 100644 (file)
--- a/Lib/profiling/sampling/stack_collector.py
+++ b/Lib/profiling/sampling/stack_collector.py
@@ -698,6 +698,8 @@ class DiffFlamegraphCollector(FlamegraphCollector):
         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_children.py b/Lib/test/test_profiling/test_sampling_profiler/test_children.py
index bb49faa890f3481dc929ce89b402dd09acc72b8a..e64d917eedde56b8326ad9b973289b5e658c0fcf 100644 (file)
--- 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."""

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 b42e7aa579f40caeb72014847cfac7a2e70786b8..390a1479fdd2975b011f5ea7e1b0b69d6d44d9d5 100644 (file)
--- a/Lib/test/test_profiling/test_sampling_profiler/test_collectors.py
+++ b/Lib/test/test_profiling/test_sampling_profiler/test_collectors.py
@@ -18,6 +18,7 @@ try:
     )
     from profiling.sampling.jsonl_collector import JsonlCollector
     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 (
@@ -82,6 +83,18 @@ class TestSampleProfilerComponents(unittest.TestCase):
         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
@@ -1403,6 +1416,39 @@ class TestSampleProfilerComponents(unittest.TestCase):
         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 = [