Skip to content
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.

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

Add option to show total time including children in flat profiles #285

Merged
merged 4 commits into from
Aug 1, 2024

Conversation

matt-graham
Copy link
Contributor

Adds a literal string argument flat_time to ConsoleRenderer which can be used to set frame time type displayed (and used as sort key) in flat profiles - "self" for total self time, excluding child frames (the default, equivalent to current behaviour) and "total" for total time, including child frames.

I find being able to view a flat list sorted by total time useful in some situations, but it does result in the less intutitive behaviour that the sum of the times shown no longer bears any relation to total run time.

Rather than a string argument could alternatively use a boolean flag if preferred or change name of options.

@matt-graham
Copy link
Contributor Author

For the remaining pyright error in pre-commit workflow, I get the same error when running pre-commit run --all-files locally on current tip of main so not sure if the failure is related to changes in this PR or a change in upstream packages?

Copy link
Owner

@joerick joerick left a comment

Choose a reason for hiding this comment

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

This looks fine to me, as-is. I don't actually use the flat renderer, but if it's useful to you, all good!

Could you add the option to Profiler.print and Profiler.output_text, too?

@matt-graham
Copy link
Contributor Author

Could you add the option to Profiler.print and Profiler.output_text, too?

flat_time option now also exposed in Profiler.print and Profiler.output_text. I created a type alias FlatTimeMode to avoid repetition of the allowable string modes in the type hint and this seemed to mirror the pattern used for AsyncMode in profiler.py but let me know if you prefer this to be named differently.

Copy link
Owner

@joerick joerick left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@joerick joerick merged commit 9eaf5c2 into joerick:main Aug 1, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants