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
1 change: 1 addition & 0 deletions Lib/profiling/sampling/binary_collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ def export(self, filename=None):
filename: Ignored (binary files are written incrementally)
"""
self._writer.finalize()
return True

@property
def total_samples(self):
Expand Down
13 changes: 9 additions & 4 deletions Lib/profiling/sampling/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -789,11 +789,12 @@ def progress_callback(current, total):
args.outfile
or _generate_output_filename(args.format, os.getpid())
)
collector.export(filename)
export_ok = collector.export(filename)

# Auto-open browser for HTML output if --browser flag is set
if (
args.format in (
export_ok
and args.format in (
'flamegraph', 'diff_flamegraph', 'heatmap'
)
and getattr(args, 'browser', False)
Expand Down Expand Up @@ -840,10 +841,14 @@ def _handle_output(collector, args, pid, mode):
filename = os.path.join(args.outfile, _generate_output_filename(args.format, pid))
else:
filename = args.outfile or _generate_output_filename(args.format, pid)
collector.export(filename)
export_ok = collector.export(filename)

# Auto-open browser for HTML output if --browser flag is set
if args.format in ('flamegraph', 'diff_flamegraph', 'heatmap') and getattr(args, 'browser', False):
if (
export_ok
and args.format in ('flamegraph', 'diff_flamegraph', 'heatmap')
and getattr(args, 'browser', False)
):
_open_in_browser(filename)


Expand Down
6 changes: 5 additions & 1 deletion Lib/profiling/sampling/collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,11 @@ def collect_failed_sample(self):

@abstractmethod
def export(self, filename):
"""Export collected data to a file."""
"""Export collected data.
Returns:
bool: True if output was generated, False if there was no data to export.
"""

@staticmethod
def _filter_internal_frames(frames):
Expand Down
1 change: 1 addition & 0 deletions Lib/profiling/sampling/gecko_collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,7 @@ def spin():
print(
f"Open in Firefox Profiler: https://profiler.firefox.com/"
)
return True

def _build_marker_schema(self):
"""Build marker schema definitions for Firefox Profiler."""
Expand Down
3 changes: 2 additions & 1 deletion Lib/profiling/sampling/heatmap_collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ def export(self, output_path):
"""
if not self.file_samples:
print("Warning: No heatmap data to export")
return
return False

try:
output_dir = self._prepare_output_directory(output_path)
Expand All @@ -611,6 +611,7 @@ def export(self, output_path):
self._generate_index_html(output_dir / 'index.html', file_stats)

self._print_export_summary(output_dir, file_stats)
return True

except Exception as e:
print(f"Error: Failed to export heatmap: {e}")
Expand Down
1 change: 1 addition & 0 deletions Lib/profiling/sampling/jsonl_collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ def export(self, filename):
self._iter_final_agg_entries(),
)
self._write_message(output, self._build_end_record())
return True

def _build_meta_record(self):
record = {
Expand Down
1 change: 1 addition & 0 deletions Lib/profiling/sampling/pstats_collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ def collect(self, stack_frames, timestamps_us=None):
def export(self, filename):
self.create_stats()
self._dump_stats(filename)
return True

def _dump_stats(self, file):
stats_with_marker = dict(self.stats)
Expand Down
4 changes: 3 additions & 1 deletion Lib/profiling/sampling/stack_collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ def export(self, filename):
for stack, count in lines:
f.write(f"{stack} {count}\n")
print(f"Collapsed stack output written to {filename}")
return True


class FlamegraphCollector(StackTraceCollector):
Expand Down Expand Up @@ -159,14 +160,15 @@ def export(self, filename):
print(
"Warning: No functions found in profiling data. Check if sampling captured any data."
)
return
return False

html_content = self._create_flamegraph_html(flamegraph_data)

with open(filename, "w", encoding="utf-8") as f:
f.write(html_content)

print(f"Flamegraph saved to: {filename}")
return True

@staticmethod
@functools.lru_cache(maxsize=None)
Expand Down
22 changes: 22 additions & 0 deletions Lib/test/test_profiling/test_sampling_profiler/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import sys
import tempfile
import unittest
from types import SimpleNamespace
from unittest import mock

try:
Expand All @@ -26,6 +27,7 @@
FORMAT_EXTENSIONS,
_create_collector,
_generate_output_filename,
_handle_output,
main,
)
from profiling.sampling.constants import (
Expand Down Expand Up @@ -727,6 +729,26 @@ def test_async_aware_flag_defaults_to_running(self):
call_kwargs = mock_sample.call_args[1]
self.assertEqual(call_kwargs.get("async_aware"), "running")

def test_handle_output_browser_not_opened_when_export_fails(self):
for format_type in ("flamegraph", "diff_flamegraph", "heatmap"):
with self.subTest(format=format_type):
collector = mock.MagicMock()
collector.export.return_value = False
args = SimpleNamespace(
format=format_type,
outfile="profile.html",
browser=True,
)

with (
mock.patch("profiling.sampling.cli.os.path.isdir", return_value=False),
mock.patch("profiling.sampling.cli._open_in_browser") as mock_open,
):
_handle_output(collector, args, pid=12345, mode=0)

collector.export.assert_called_once_with("profile.html")
mock_open.assert_not_called()

def test_async_aware_with_async_mode_all(self):
"""Test --async-aware with --async-mode all."""
test_args = ["profiling.sampling.cli", "attach", "12345", "--async-aware", "--async-mode", "all"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -502,9 +502,10 @@ def test_flamegraph_collector_export(self):

# Export flamegraph
with captured_stdout(), captured_stderr():
collector.export(flamegraph_out.name)
export_ok = collector.export(flamegraph_out.name)

# Verify file was created and contains valid data
self.assertTrue(export_ok)
self.assertTrue(os.path.exists(flamegraph_out.name))
self.assertGreater(os.path.getsize(flamegraph_out.name), 0)

Expand All @@ -523,6 +524,21 @@ def test_flamegraph_collector_export(self):
self.assertIn('"value":', content)
self.assertIn('"children":', content)

def test_flamegraph_collector_empty_export_fails(self):
"""Test empty flamegraph export reports no output."""
flamegraph_out = tempfile.NamedTemporaryFile(
suffix=".html", delete=False
)
self.addCleanup(close_and_unlink, flamegraph_out)

collector = FlamegraphCollector(1000)

with captured_stdout(), captured_stderr():
export_ok = collector.export(flamegraph_out.name)

self.assertFalse(export_ok)
self.assertEqual(os.path.getsize(flamegraph_out.name), 0)

def test_gecko_collector_basic(self):
"""Test basic GeckoCollector functionality."""
collector = GeckoCollector(1000)
Expand Down Expand Up @@ -1552,8 +1568,9 @@ def test_diff_flamegraph_export(self):
self.addCleanup(close_and_unlink, flamegraph_out)

with captured_stdout(), captured_stderr():
diff.export(flamegraph_out.name)
export_ok = diff.export(flamegraph_out.name)

self.assertTrue(export_ok)
self.assertTrue(os.path.exists(flamegraph_out.name))
self.assertGreater(os.path.getsize(flamegraph_out.name), 0)

Expand Down
Loading