diff --git a/Lib/profiling/sampling/stack_collector.py b/Lib/profiling/sampling/stack_collector.py index eb1a3fba93cf33b..90dba3558590341 100644 --- a/Lib/profiling/sampling/stack_collector.py +++ b/Lib/profiling/sampling/stack_collector.py @@ -592,9 +592,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: @@ -614,6 +638,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 @@ -690,6 +722,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")) @@ -705,9 +740,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 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 7746811014a9e2f..1fdd57911f0007e 100644 --- a/Lib/test/test_profiling/test_sampling_profiler/test_collectors.py +++ b/Lib/test/test_profiling/test_sampling_profiler/test_collectors.py @@ -1664,6 +1664,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 = [ diff --git a/Misc/NEWS.d/next/Library/2026-07-19-12-00-00.gh-issue-154085.Qm8fLd.rst b/Misc/NEWS.d/next/Library/2026-07-19-12-00-00.gh-issue-154085.Qm8fLd.rst new file mode 100644 index 000000000000000..40ab41a2f296995 --- /dev/null +++ b/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.