Skip to content

Add flat/cumulative function summary to handle recursion-heavy profiles #430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
huyhoang171106 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add flat/cumulative function summary to handle recursion-heavy profiles #430

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
53 changes: 52 additions & 1 deletion pyinstrument/renderers/console.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ def __init__(
unicode: bool = False,
color: bool = False,
flat: bool = False,
show_top: bool = False,
time: LiteralStr["seconds", "percent_of_total"] = "seconds",
flat_time: FlatTimeMode = "self",
short_mode: bool = False,
Expand All @@ -41,6 +42,7 @@ def __init__(
:param unicode: Use unicode, like box-drawing characters in the output.
:param color: Enable color support, using ANSI color sequences.
:param flat: Display a flat profile instead of a call graph.
:param show_top: Display an aggregated top-functions table.
:param time: How to display the duration of each frame - ``'seconds'`` or ``'percent_of_total'``
:param flat_time: Show ``'self'`` time or ``'total'`` time (including children) in flat profile.
:param short_mode: Display a short version of the output.
Expand All @@ -52,13 +54,17 @@ def __init__(
self.unicode = unicode
self.color = color
self.flat = flat
self.show_top = show_top
self.time = time
self.flat_time = flat_time
self.short_mode = short_mode

if self.flat and self.timeline:
raise Renderer.MisconfigurationError("Cannot use timeline and flat options together.")

if self.flat and self.show_top:
raise Renderer.MisconfigurationError("Cannot use flat and show_top options together.")

Comment on lines 33 to +67
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A new show_top option was added to ConsoleRenderer, but it isn’t currently exposed through Profiler.print() / Profiler.output_text() (and the CLI option plumbing appears absent). This will also break test_profiler_convenience_methods_have_all_options_available (it asserts Profiler convenience methods mirror renderer signatures). Add show_top (with matching default/annotation) to the Profiler convenience methods and any option parsing that constructs a ConsoleRenderer.

Copilot uses AI. Check for mistakes.
self.colors = self.colors_enabled if color else self.colors_disabled

def render(self, session: Session) -> str:
Expand All @@ -73,7 +79,9 @@ def render(self, session: Session) -> str:
else:
self.root_frame = frame

if self.flat:
if self.show_top:
result += self.render_top_summary(self.root_frame, precision=precision, indent=indent)
elif self.flat:
result += self.render_frame_flat(self.root_frame, precision=precision)
else:
Comment on lines +82 to 86
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new show_top rendering path isn’t covered by tests (existing tests cover empty profiles, timeline, show_all, and flat modes). Add a renderer test that exercises ConsoleRenderer(show_top=True) and asserts the output format/ordering (at least that it doesn’t crash and includes the expected header).

Copilot uses AI. Check for mistakes.
result += self.render_frame(
Expand Down Expand Up @@ -161,6 +169,49 @@ def libraries_for_frames(self, frames: list[Frame]) -> list[str]:
libraries.append(library)
return libraries

def render_top_summary(self, frame: Frame, precision: int, indent: str = "") -> str:
aggregated: Dict[Tuple[str, str], Dict[str, float]] = {}
frames = [frame]

while frames:
current = frames.pop()
function = current.function or "<unknown>"
location = current.code_position_short or ""
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frame.code_position_short is a method, but here it’s being used as an attribute (current.code_position_short). This will make location a bound-method object (always truthy), producing unusable keys/labels (e.g., printing the method repr) and breaking aggregation. Call current.code_position_short() instead.

Suggested change
location = current.code_position_short or ""
location = current.code_position_short() or ""

Copilot uses AI. Check for mistakes.
key = (function, location)

if key not in aggregated:
aggregated[key] = {"self_time": 0.0, "total_time": 0.0}

aggregated[key]["self_time"] += current.total_self_time
aggregated[key]["total_time"] += current.time

Comment on lines +184 to +187
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

total_time is being aggregated by summing current.time for every frame occurrence. For recursive call stacks, inclusive times overlap across nested invocations, so this will significantly over-count (and can exceed the root total), making the new “top functions” view misleading for the recursion-heavy use case this PR targets. Consider defining “total” in a way that avoids double-counting recursion (e.g., subtract time from recursive children with the same key, or compute cumulative time on a call graph with cycle handling, similar to pprof).

Copilot uses AI. Check for mistakes.
frames.extend(current.children)

sorted_rows = sorted(
aggregated.items(), key=lambda item: (item[1]["total_time"], item[1]["self_time"]), reverse=True
)

if not sorted_rows:
return f"{indent}No samples were recorded.\n"

header = f"{indent}{'Self':>10} {'Total':>10} Function"
lines = [header]

for (function, location), times in sorted_rows:
self_time = self._format_top_time(times["self_time"], frame.time, precision)
total_time = self._format_top_time(times["total_time"], frame.time, precision)
label = function if not location else f"{function} ({location})"
lines.append(f"{indent}{self_time:>10} {total_time:>10} {label}")

Comment on lines +190 to +205
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

render_top_summary() prints every aggregated row unconditionally. For large profiles this can produce extremely large outputs, and it also ignores the existing show_all behavior used by render_frame_flat() (0.1% threshold). Consider adding a default filter/limit (e.g., drop entries below a % threshold or show top N) and let show_all=True disable that filter.

Copilot uses AI. Check for mistakes.
return "\n".join(lines) + "\n"

def _format_top_time(self, value: float, total: float, precision: int) -> str:
if self.time == "percent_of_total":
if total <= 0:
return "0.0%"
return f"{(100 * value / total):.{precision}f}%"
return f"{value:.{precision}f}s"

def render_frame(
self, frame: Frame, precision: int, indent: str = "", child_indent: str = ""
) -> str:
Expand Down
Loading