Update profiling panel to use an absolute cumtime comparison. - #2447
Open
tim-schilling wants to merge 3 commits into
Open
Update profiling panel to use an absolute cumtime comparison.#2447tim-schilling wants to merge 3 commits into
tim-schilling wants to merge 3 commits into
Conversation
This reworks ProfilingPanel to contain a dedicated method to manage whether a function call should be included in the profiling stats results. This moves the depth down to allow developers to customize this at the expense of duplicating the comparison. This retires PROFILER_THRESHOLD_RATIO as it didn't work super well.
The all_callees is a graph data structure and needs to be traversed entirely to reconstruct the callers tree. This is why we need to skip subsequent nodes since the time was already aggregated into the first call appearance.
Coverage Report
File CoverageNo changed files found. |
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This reworks
ProfilingPanelto contain a dedicated method to manage whether a function call should be included in the profiling stats results. This moves the depth down to allow developers to customize this at the expense of duplicating the comparison. It also uses the aggregate timing values rather than an individual callers timings. This is what truly fixes #2209. This also revealed thatall_calleeswas a graph. This meant that function calls may appear more than once and would need to be skipped since we've already aggregated those times. It appears this also helps with #1004 (I was able to turn the max depth up way higher than usual).This retires
PROFILER_THRESHOLD_RATIOas it didn't work super well.Fixes #2209
Checklist:
docs/changes.rst.AI/LLM Usage
I used Claude to determine why
pStatswas able to collect data differently than our implementation.