diff --git a/Lib/profiling/sampling/stack_collector.py b/Lib/profiling/sampling/stack_collector.py index e420bb6d2e9b87..72a8cae8807890 100644 --- a/Lib/profiling/sampling/stack_collector.py +++ b/Lib/profiling/sampling/stack_collector.py @@ -676,16 +676,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 @@ -902,6 +902,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) diff --git a/Lib/test/test_profiling/test_sampling_profiler/mocks.py b/Lib/test/test_profiling/test_sampling_profiler/mocks.py index 6ac2d08e898d81..128870ffd4d5e4 100644 --- a/Lib/test/test_profiling/test_sampling_profiler/mocks.py +++ b/Lib/test/test_profiling/test_sampling_profiler/mocks.py @@ -93,7 +93,9 @@ 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 ( @@ -101,12 +103,14 @@ def make_diff_collector_with_mock_baseline(baseline_samples): 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 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 eb58c29dd361d3..36713ff1314ff9 100644 --- a/Lib/test/test_profiling/test_sampling_profiler/test_collectors.py +++ b/Lib/test/test_profiling/test_sampling_profiler/test_collectors.py @@ -1663,7 +1663,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] ) @@ -1673,7 +1674,7 @@ 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") @@ -1681,17 +1682,17 @@ def test_diff_flamegraph_changed_functions(self): 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 @@ -1737,8 +1738,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, [ @@ -1753,15 +1754,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.""" @@ -2064,7 +2124,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() @@ -2074,6 +2138,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.""" @@ -2240,7 +2309,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) @@ -2270,7 +2340,7 @@ 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") @@ -2278,17 +2348,17 @@ def test_diff_flamegraph_load_baseline(self): 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) diff --git a/Misc/NEWS.d/next/Library/2026-07-19-11-00-00.gh-issue-154062.N5ks3A.rst b/Misc/NEWS.d/next/Library/2026-07-19-11-00-00.gh-issue-154062.N5ks3A.rst new file mode 100644 index 00000000000000..7c4469aba9bc7e --- /dev/null +++ b/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.