Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 19 additions & 9 deletions Lib/profiling/sampling/stack_collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -550,16 +550,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

Expand Down Expand Up @@ -653,7 +653,10 @@ def _build_elided_flamegraph(self, baseline_stats, scale):
if not self._extract_elided_nodes(baseline_data, path=()):
return None

# Metadata is calculated from raw baseline sample counts. Scale the
# rendered geometry only after those counts have been annotated.
self._add_elided_metadata(baseline_data, baseline_stats, scale, path=())
self._scale_flamegraph_values(baseline_data, scale)

# Merge only profiling metadata, not thread-level stats
for key in ("sample_interval_usec", "duration_sec", "sample_rate",
Expand All @@ -666,6 +669,13 @@ def _build_elided_flamegraph(self, baseline_stats, scale):

return baseline_data

def _scale_flamegraph_values(self, node, scale):
"""Express flamegraph values in units of the current sample interval."""
node["value"] = node.get("value", 0) * scale
node["self"] = node.get("self", 0) * scale
for child in node.get("children", ()):
self._scale_flamegraph_values(child, scale)

def _extract_elided_nodes(self, node, path):
"""Remove non-elided nodes and recalculate values bottom-up."""
if not node:
Expand Down
10 changes: 7 additions & 3 deletions Lib/test/test_profiling/test_sampling_profiler/mocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
103 changes: 77 additions & 26 deletions Lib/test/test_profiling/test_sampling_profiler/test_collectors.py
Original file line number Diff line number Diff line change
Expand Up @@ -1500,7 +1500,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]
)
Expand All @@ -1510,28 +1511,28 @@ 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_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, [
Expand All @@ -1546,15 +1547,64 @@ 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")])
])
]
current_frames = [
MockInterpreterInfo(0, [
MockThreadInfo(1, [MockFrameInfo("file.py", 20, "new_func")])
])
]

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.assertAlmostEqual(elided["self"], 1.0)
self.assertAlmostEqual(elided["baseline"], 1.0)
self.assertAlmostEqual(elided["diff"], -1.0)

def test_diff_flamegraph_elided_stacks(self):
"""Paths in baseline but not current produce elided stacks."""
Expand Down Expand Up @@ -1848,7 +1898,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)

Expand Down Expand Up @@ -1878,25 +1929,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)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Stop normalizing differential flamegraph baselines to the duration of the
current profile.
Loading