[3.15] gh-154085: Avoid duplicating diff line values (GH-154099) (#156385)

gh-154085: Avoid duplicating diff line values (GH-154099)
(cherry picked from commit 53381bc)

Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>

Author: miss-islington

Lib/profiling/sampling/stack_collector.py

@@ -675,9 +675,33 @@ def _add_diff_data_to_node(self, node, path, current_stats, baseline_stats, scal
         current_data = current_stats.get(path_key, {"total": 0, "self": 0})
         baseline_data = baseline_stats.get(path_key, {"total": 0, "self": 0})
 
-        current_self = current_data["self"]
-        baseline_self = baseline_data["self"] * scale
-        baseline_total = baseline_data["total"] * scale
+        current_self = node.get("self", 0)
+        current_total = node.get("value", 0)
+
+        current_nonself = current_total - current_self
+        aggregate_nonself = current_data["total"] - current_data["self"]
+
+        # Allocate self and descendant samples separately.  Line-number
+        # changes can split one function path into several rendered nodes,
+        # and using independent weights for self and inclusive totals could
+        # otherwise assign a node more self samples than total samples.
+        self_weight = self._sample_weight(
+            current_self,
+            current_data["self"],
+            current_total,
+            current_data["total"],
+        )
+        nonself_weight = self._sample_weight(
+            current_nonself,
+            aggregate_nonself,
+            current_total,
+            current_data["total"],
+        )
+        baseline_self = baseline_data["self"] * scale * self_weight
+        baseline_nonself = (
+            baseline_data["total"] - baseline_data["self"]
+        ) * scale * nonself_weight
+        baseline_total = baseline_self + baseline_nonself
 
         diff = current_self - baseline_self
         if baseline_self > 0:
@@ -697,6 +721,14 @@ def _add_diff_data_to_node(self, node, path, current_stats, baseline_stats, scal
             for child in node["children"]:
                 self._add_diff_data_to_node(child, path_key, current_stats, baseline_stats, scale)
 
+    @staticmethod
+    def _sample_weight(value, aggregate, fallback_value, fallback_aggregate):
+        if aggregate > 0:
+            return value / aggregate
+        if fallback_aggregate > 0:
+            return fallback_value / fallback_aggregate
+        return 0
+
     def _is_promoted_root(self, data):
         """Check if the data represents a promoted root node."""
         return "filename" in data and "funcname" in data
@@ -783,6 +815,9 @@ def _extract_elided_nodes(self, node, path):
             # elided nodes keep their original value to preserve self-samples
             if elided_children and not is_elided:
                 node["value"] = total_value
+                node["self"] = 0
+                node.pop("opcodes", None)
+                node.pop("thread_opcodes", None)
 
         # Keep this node if it's elided or has elided descendants
         return is_elided or bool(node.get("children"))
@@ -798,9 +833,13 @@ def _add_elided_metadata(self, node, baseline_stats, scale, 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
-            baseline_total = baseline_data["total"] * scale
+            baseline_total = node.get("value", 0) * scale
+
+            # Matched nodes are retained only as structural ancestors.  Their
+            # own samples are still present in the current profile and must
+            # not be reported as disappeared.
+            if current_path in self._elided_paths:
+                baseline_self = node.get("self", 0) * scale
 
             node["baseline"] = baseline_self
             node["baseline_total"] = baseline_total

Lib/test/test_profiling/test_sampling_profiler/test_collectors.py

@@ -1840,6 +1840,123 @@ def test_diff_flamegraph_function_matched_despite_line_change(self):
         self.assertAlmostEqual(child["diff"], 0.0, places=1)
         self.assertAlmostEqual(child["diff_pct"], 0.0, places=1)
 
+    def test_diff_flamegraph_does_not_duplicate_line_values(self):
+        """Function aggregates are apportioned across line nodes."""
+        def sample(line):
+            return [
+                MockInterpreterInfo(0, [
+                    MockThreadInfo(1, [
+                        MockFrameInfo("file.py", line, "func"),
+                        MockFrameInfo("file.py", 1, "caller"),
+                    ])
+                ])
+            ]
+
+        diff = make_diff_collector_with_mock_baseline(
+            [sample(10), sample(20)]
+        )
+        diff.collect(sample(10))
+        diff.collect(sample(20))
+
+        data = diff._convert_to_flamegraph_format()
+        children = data["children"]
+        self.assertEqual(sum(node["self"] for node in children), 2)
+        self.assertEqual(sum(node["self_time"] for node in children), 2)
+        self.assertEqual(sum(node["baseline"] for node in children), 2)
+        for node in children:
+            self.assertEqual(node["self"], 1)
+            self.assertEqual(node["self_time"], 1)
+            self.assertAlmostEqual(node["baseline"], 1.0)
+            self.assertAlmostEqual(node["diff"], 0.0)
+
+    def test_diff_flamegraph_line_totals_include_allocated_self(self):
+        """A line's baseline self time cannot exceed its inclusive time."""
+        def sample(*frames):
+            return [
+                MockInterpreterInfo(0, [MockThreadInfo(1, list(frames))])
+            ]
+
+        target_10 = MockFrameInfo("file.py", 10, "target")
+        target_20 = MockFrameInfo("file.py", 20, "target")
+        child = MockFrameInfo("file.py", 30, "child")
+
+        diff = make_diff_collector_with_mock_baseline(
+            [sample(target_10)] * 100
+        )
+        for _ in range(10):
+            diff.collect(sample(target_10))
+        for _ in range(90):
+            diff.collect(sample(child, target_20))
+
+        data = diff._convert_to_flamegraph_format()
+        nodes = data["children"]
+        self.assertEqual(sum(node["baseline"] for node in nodes), 100)
+        self.assertEqual(sum(node["baseline_total"] for node in nodes), 100)
+        for node in nodes:
+            self.assertGreaterEqual(node["baseline"], 0)
+            self.assertLessEqual(node["baseline"], node["baseline_total"])
+
+    def test_diff_flamegraph_does_not_duplicate_elided_line_values(self):
+        """Elided metadata uses each rendered line node's samples."""
+        def sample(line, funcname="old_func"):
+            return [
+                MockInterpreterInfo(0, [
+                    MockThreadInfo(1, [
+                        MockFrameInfo("file.py", line, funcname),
+                        MockFrameInfo("file.py", 1, "caller"),
+                    ])
+                ])
+            ]
+
+        diff = make_diff_collector_with_mock_baseline(
+            [sample(10), sample(20)]
+        )
+        diff.collect(sample(30, "new_func"))
+
+        data = diff._convert_to_flamegraph_format()
+        elided = data["stats"]["elided_flamegraph"]
+        children = elided["children"]
+        scale = data["stats"]["baseline_scale"]
+        self.assertEqual(sum(node["self"] for node in children), 2)
+        self.assertEqual(sum(node["baseline"] for node in children), 2 * scale)
+        for node in children:
+            self.assertEqual(node["self"], 1)
+            self.assertAlmostEqual(node["baseline"], scale)
+            self.assertAlmostEqual(node["diff"], -scale)
+
+    def test_diff_flamegraph_elided_ancestors_have_no_lost_self_time(self):
+        """Matched ancestors only carry inclusive elided geometry."""
+        root = MockFrameInfo("file.py", 10, "root")
+        common = MockFrameInfo("file.py", 20, "common", opcode=100)
+        old = MockFrameInfo("file.py", 30, "old")
+
+        common_sample = [
+            MockInterpreterInfo(0, [MockThreadInfo(1, [common, root])])
+        ]
+        old_sample = [
+            MockInterpreterInfo(0, [MockThreadInfo(1, [old, common, root])])
+        ]
+
+        diff = make_diff_collector_with_mock_baseline(
+            [common_sample] * 3 + [old_sample]
+        )
+        diff.collect(common_sample)
+
+        data = diff._convert_to_flamegraph_format()
+        elided_root = data["stats"]["elided_flamegraph"]
+        common_node = elided_root["children"][0]
+        old_node = common_node["children"][0]
+
+        for ancestor in (elided_root, common_node):
+            self.assertEqual(ancestor["self"], 0)
+            self.assertEqual(ancestor["baseline"], 0)
+            self.assertNotIn("opcodes", ancestor)
+            self.assertLessEqual(
+                ancestor["baseline"], ancestor["baseline_total"]
+            )
+        self.assertEqual(old_node["self"], 1)
+        self.assertEqual(old_node["baseline"], old_node["baseline_total"])
+
     def test_diff_flamegraph_empty_current(self):
         """Empty current profile still produces differential metadata and elided paths."""
         baseline_frames = [

Misc/NEWS.d/next/Library/2026-07-19-12-00-00.gh-issue-154085.Qm8fLd.rst

@@ -0,0 +1,2 @@
+Prevent differential flamegraphs from duplicating self time across line
+nodes for the same function.