[3.15] gh-154062: Stop normalizing differential flamegraph duration (GH-154082) (#158871)

Author: pablogsal

Lib/profiling/sampling/stack_collector.py

@@ -681,16 +681,16 @@ def _convert_to_flamegraph_format(self):
         current_stats = self._aggregate_path_samples(self._root)
         baseline_stats = self._aggregate_path_samples(self._baseline_collector._root)
 
-        # Scale baseline values to make them comparable, accounting for both
-        # sample count differences and sample interval differences.
+        # Express baseline samples in units of the current sample interval.
+        # Do not normalize by total profile duration: doing so makes unchanged
+        # functions appear different when another function becomes faster or
+        # slower.
         baseline_total = self._baseline_collector._total_samples
-        if baseline_total > 0 and self._total_samples > 0:
-            current_time = self._total_samples * self.sample_interval_usec
-            baseline_time = baseline_total * self._baseline_collector.sample_interval_usec
-            scale = current_time / baseline_time
-        elif baseline_total > 0:
-            # Current profile is empty - use interval-based scale for elided display
-            scale = self.sample_interval_usec / self._baseline_collector.sample_interval_usec
+        if baseline_total > 0:
+            scale = (
+                self._baseline_collector.sample_interval_usec
+                / self.sample_interval_usec
+            )
         else:
             scale = 1.0
 
@@ -907,6 +907,10 @@ def _add_elided_metadata(self, node, baseline_stats, scale, path):
         else:
             node["diff_pct"] = 0.0
 
+        # Scale geometry after computing metadata from raw baseline counts.
+        node["value"] = node.get("value", 0) * scale
+        node["self"] = node.get("self", 0) * scale
+
         if "children" in node and node["children"]:
             for child in node["children"]:
                 self._add_elided_metadata(child, baseline_stats, scale, current_path)

Lib/test/test_profiling/test_sampling_profiler/mocks.py

@@ -93,20 +93,24 @@ def __repr__(self):
         return f"MockAwaitedInfo(thread_id={self.thread_id}, awaited_by={len(self.awaited_by)} tasks)"
 
 
-def make_diff_collector_with_mock_baseline(baseline_samples):
+def make_diff_collector_with_mock_baseline(
+    baseline_samples, *, baseline_interval=1000, current_interval=1000
+):
     """Create a DiffFlamegraphCollector with baseline injected directly,
     skipping the binary round-trip that _load_baseline normally does."""
     from profiling.sampling.stack_collector import (
         DiffFlamegraphCollector,
         FlamegraphCollector,
     )
 
-    baseline = FlamegraphCollector(1000)
+    baseline = FlamegraphCollector(baseline_interval)
     for sample in baseline_samples:
         baseline.collect(sample)
 
     # Path is unused since we inject _baseline_collector directly;
     # use __file__ as a dummy path that passes the existence check.
-    diff = DiffFlamegraphCollector(1000, baseline_binary_path=__file__)
+    diff = DiffFlamegraphCollector(
+        current_interval, baseline_binary_path=__file__
+    )
     diff._baseline_collector = baseline
     return diff

Lib/test/test_profiling/test_sampling_profiler/test_collectors.py

@@ -1664,7 +1664,8 @@ def test_diff_flamegraph_changed_functions(self):
             ])
         ]
 
-        # Baseline: 2 samples, current: 4, scale = 2.0
+        # Baseline: 2 samples, current: 4. Profiles are compared in absolute
+        # time rather than normalized to the same total duration.
         diff = make_diff_collector_with_mock_baseline(
             [hot_leaf_sample, cold_leaf_sample]
         )
@@ -1674,25 +1675,25 @@ def test_diff_flamegraph_changed_functions(self):
 
         data = diff._convert_to_flamegraph_format()
         strings = data.get("strings", [])
-        self.assertAlmostEqual(data["stats"]["baseline_scale"], 2.0)
+        self.assertAlmostEqual(data["stats"]["baseline_scale"], 1.0)
 
         children = data.get("children", [])
         hot_node = find_child_by_name(children, strings, "hot_leaf")
         cold_node = find_child_by_name(children, strings, "cold_leaf")
         self.assertIsNotNone(hot_node)
         self.assertIsNotNone(cold_node)
 
-        # hot_leaf regressed (+50%)
-        self.assertAlmostEqual(hot_node["baseline"], 2.0)
+        # hot_leaf regressed (+200%)
+        self.assertAlmostEqual(hot_node["baseline"], 1.0)
         self.assertEqual(hot_node["self_time"], 3)
-        self.assertAlmostEqual(hot_node["diff"], 1.0)
-        self.assertAlmostEqual(hot_node["diff_pct"], 50.0)
+        self.assertAlmostEqual(hot_node["diff"], 2.0)
+        self.assertAlmostEqual(hot_node["diff_pct"], 200.0)
 
-        # cold_leaf improved (-50%)
-        self.assertAlmostEqual(cold_node["baseline"], 2.0)
+        # cold_leaf is unchanged
+        self.assertAlmostEqual(cold_node["baseline"], 1.0)
         self.assertEqual(cold_node["self_time"], 1)
-        self.assertAlmostEqual(cold_node["diff"], -1.0)
-        self.assertAlmostEqual(cold_node["diff_pct"], -50.0)
+        self.assertAlmostEqual(cold_node["diff"], 0.0)
+        self.assertAlmostEqual(cold_node["diff_pct"], 0.0)
 
     def test_diff_flamegraph_rejects_mismatched_profiling_modes(self):
         from profiling.sampling.binary_collector import BinaryCollector
@@ -1738,8 +1739,8 @@ def test_diff_flamegraph_rejects_mismatched_capture_config(self):
         with self.assertRaisesRegex(ValueError, "all_threads"):
             diff._convert_to_flamegraph_format()
 
-    def test_diff_flamegraph_scale_factor(self):
-        """Scale factor adjusts when sample counts differ."""
+    def test_diff_flamegraph_does_not_normalize_duration(self):
+        """A longer current run is compared in absolute time."""
         baseline_frames = [
             MockInterpreterInfo(0, [
                 MockThreadInfo(1, [
@@ -1754,15 +1755,74 @@ def test_diff_flamegraph_scale_factor(self):
             diff.collect(baseline_frames)
 
         data = diff._convert_to_flamegraph_format()
-        self.assertAlmostEqual(data["stats"]["baseline_scale"], 4.0)
+        self.assertAlmostEqual(data["stats"]["baseline_scale"], 1.0)
 
         children = data.get("children", [])
         self.assertEqual(len(children), 1)
         func1_node = children[0]
         self.assertEqual(func1_node["self_time"], 4)
-        self.assertAlmostEqual(func1_node["baseline"], 4.0)
-        self.assertAlmostEqual(func1_node["diff"], 0.0)
-        self.assertAlmostEqual(func1_node["diff_pct"], 0.0)
+        self.assertAlmostEqual(func1_node["baseline"], 1.0)
+        self.assertAlmostEqual(func1_node["diff"], 3.0)
+        self.assertAlmostEqual(func1_node["diff_pct"], 300.0)
+
+    def test_diff_flamegraph_scale_factor_uses_sample_intervals(self):
+        """Baseline samples are converted to current sample units."""
+        frames = [
+            MockInterpreterInfo(0, [
+                MockThreadInfo(1, [MockFrameInfo("file.py", 10, "func1")])
+            ])
+        ]
+
+        diff = make_diff_collector_with_mock_baseline(
+            [frames] * 10,
+            baseline_interval=1000,
+            current_interval=10000,
+        )
+        diff.collect(frames)
+
+        data = diff._convert_to_flamegraph_format()
+        self.assertAlmostEqual(data["stats"]["baseline_scale"], 0.1)
+        self.assertAlmostEqual(data["baseline"], 1.0)
+        self.assertEqual(data["self_time"], 1)
+        self.assertAlmostEqual(data["diff"], 0.0)
+        self.assertAlmostEqual(data["diff_pct"], 0.0)
+
+    def test_diff_flamegraph_elided_values_use_current_interval(self):
+        """Elided geometry and metadata use the same sample units."""
+        baseline_frames = [
+            MockInterpreterInfo(0, [
+                MockThreadInfo(1, [
+                    MockFrameInfo("file.py", 10, "old_func"),
+                    MockFrameInfo("file.py", 30, "parent"),
+                ])
+            ])
+        ]
+        current_frames = [
+            MockInterpreterInfo(0, [
+                MockThreadInfo(1, [
+                    MockFrameInfo("file.py", 20, "new_func"),
+                    MockFrameInfo("file.py", 30, "parent"),
+                ])
+            ])
+        ]
+
+        diff = make_diff_collector_with_mock_baseline(
+            [baseline_frames] * 10,
+            baseline_interval=1000,
+            current_interval=10000,
+        )
+        diff.collect(current_frames)
+
+        data = diff._convert_to_flamegraph_format()
+        elided = data["stats"]["elided_flamegraph"]
+        self.assertAlmostEqual(elided["value"], 1.0)
+        self.assertEqual(elided["self"], 0)
+        self.assertAlmostEqual(elided["baseline_total"], 1.0)
+        child, = elided["children"]
+        self.assertAlmostEqual(child["value"], 1.0)
+        self.assertAlmostEqual(child["self"], 1.0)
+        self.assertAlmostEqual(child["baseline"], 1.0)
+        self.assertAlmostEqual(child["diff"], -1.0)
 
     def test_diff_flamegraph_elided_stacks(self):
         """Paths in baseline but not current produce elided stacks."""
@@ -2065,7 +2125,11 @@ def test_diff_flamegraph_empty_current(self):
             ])
         ]
 
-        diff = make_diff_collector_with_mock_baseline([baseline_frames])
+        diff = make_diff_collector_with_mock_baseline(
+            [baseline_frames] * 10,
+            baseline_interval=1000,
+            current_interval=10000,
+        )
         # Don't collect anything in current
 
         data = diff._convert_to_flamegraph_format()
@@ -2075,6 +2139,11 @@ def test_diff_flamegraph_empty_current(self):
         self.assertTrue(data["stats"]["is_differential"])
         # All baseline paths should be elided since current is empty
         self.assertGreater(data["stats"]["elided_count"], 0)
+        self.assertAlmostEqual(data["stats"]["baseline_scale"], 0.1)
+        elided = data["stats"]["elided_flamegraph"]
+        self.assertAlmostEqual(elided["value"], 1.0)
+        self.assertAlmostEqual(elided["baseline"], 1.0)
+        self.assertAlmostEqual(elided["diff"], -1.0)
 
     def test_diff_flamegraph_empty_baseline(self):
         """Empty baseline with non-empty current uses scale=1.0 fallback."""
@@ -2241,7 +2310,8 @@ def test_diff_flamegraph_load_baseline(self):
             make_frame("file.py", 20, "caller"),
         ])])]
 
-        # Baseline: 2 samples, current: 4, scale = 2.0
+        # Baseline: 2 samples, current: 4. Profiles are compared in absolute
+        # time rather than normalized to the same total duration.
         bin_file = tempfile.NamedTemporaryFile(suffix=".bin", delete=False)
         self.addCleanup(close_and_unlink, bin_file)
 
@@ -2271,25 +2341,25 @@ def test_diff_flamegraph_load_baseline(self):
         strings = data.get("strings", [])
 
         self.assertTrue(data["stats"]["is_differential"])
-        self.assertAlmostEqual(data["stats"]["baseline_scale"], 2.0)
+        self.assertAlmostEqual(data["stats"]["baseline_scale"], 1.0)
 
         children = data.get("children", [])
         hot_node = find_child_by_name(children, strings, "hot_leaf")
         cold_node = find_child_by_name(children, strings, "cold_leaf")
         self.assertIsNotNone(hot_node)
         self.assertIsNotNone(cold_node)
 
-        # hot_leaf regressed (+50%)
-        self.assertAlmostEqual(hot_node["baseline"], 2.0)
+        # hot_leaf regressed (+200%)
+        self.assertAlmostEqual(hot_node["baseline"], 1.0)
         self.assertEqual(hot_node["self_time"], 3)
-        self.assertAlmostEqual(hot_node["diff"], 1.0)
-        self.assertAlmostEqual(hot_node["diff_pct"], 50.0)
+        self.assertAlmostEqual(hot_node["diff"], 2.0)
+        self.assertAlmostEqual(hot_node["diff_pct"], 200.0)
 
-        # cold_leaf improved (-50%)
-        self.assertAlmostEqual(cold_node["baseline"], 2.0)
+        # cold_leaf is unchanged
+        self.assertAlmostEqual(cold_node["baseline"], 1.0)
         self.assertEqual(cold_node["self_time"], 1)
-        self.assertAlmostEqual(cold_node["diff"], -1.0)
-        self.assertAlmostEqual(cold_node["diff_pct"], -50.0)
+        self.assertAlmostEqual(cold_node["diff"], 0.0)
+        self.assertAlmostEqual(cold_node["diff_pct"], 0.0)
 
     def test_jsonl_collector_export_exact_output(self):
         jsonl_out = tempfile.NamedTemporaryFile(delete=False)

Misc/NEWS.d/next/Library/2026-07-19-11-00-00.gh-issue-154062.N5ks3A.rst

@@ -0,0 +1,2 @@
+Stop normalizing differential flamegraph baselines to the duration of the
+current profile.