Skip to content

ui: Add self-time (aka self-duration) to slice aggregation - #2946

Merged
stevegolton merged 2 commits into
mainfrom
dev/sg/self-time
Sep 15, 2025
Merged

stevegolton merged 2 commits into
mainfrom
dev/sg/self-time

Conversation

@stevegolton

@stevegolton stevegolton commented Sep 15, 2025 •

Copy link
Copy Markdown
Member

This patch adds a new column in the slice aggregation table for 'Self duration', which is the time a slice was running minus the total time any of its children were running.

Note: The additional column adds around 25% overhead to how long it takes to calculate the entire aggregation, which is reasonable considering the the extra functionality.

image

Change-Id: Ie3500362519291ce741fd83f5905840775458631
@github-actions

Copy link
Copy Markdown

🤖 Hi @stevegolton, I've received your request, and I'm working on it now! You can track my progress in the logs for more details.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📋 Review Summary

This pull request introduces a "self-duration" calculation for aggregated slice views. The implementation looks solid and correctly calculates self-time by subtracting the sum of child slice durations from the parent's duration.

🔍 General Feedback

  • The use of a temporary table to pre-calculate child durations is a clean and efficient approach.
  • The changes in trace_processor_slice_track.ts to include parent_id are straightforward and necessary for this feature.
  • I've added one minor comment regarding SQL formatting for consistency.

Overall, this is a great addition that will provide more detailed performance insights.

@LalitMaganti LalitMaganti left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Awesome! So cool! LGTM :)

@LalitMaganti

Copy link
Copy Markdown
Member

Can we also mark #1256 as fixed?

@stevegolton

stevegolton commented Sep 15, 2025 •

Copy link
Copy Markdown
Member Author

Benchmarks seem to about double the time taken to calculate the aggregation (less the time taken to build the initial ii table). I'll take a look tomorrow to see if there are any improvements we can make before landing.

Edit: With the ii table creation it seems to only take about 25% longer. I think this is probably worth it for the extra functionality.

@stevegolton
stevegolton merged commit abdeafd into main Sep 15, 2025
23 checks passed
@stevegolton
stevegolton deleted the dev/sg/self-time branch September 15, 2025 20:07
stevegolton added a commit that referenced this pull request Oct 20, 2025
…3270)

Since #2946, slice tracks must contain a `parent_id` column in order to
be considered for slice aggregation, which rules out some of the
slice-like plugin tracks that are generated from a query and don't
necessarily have a parent_id in their data source or are tracks with no
concept of a stack or depth.

This patch relaxes the parent_id constraint, allowing tracks both with
and without parent_id to be aggregated.

Fixes https://buganizer.corp.google.com/issues/448068359
stevegolton added a commit that referenced this pull request Oct 20, 2025
…3270)

Since #2946, slice tracks must contain a `parent_id` column in order to
be considered for slice aggregation, which rules out some of the
slice-like plugin tracks that are generated from a query and don't
necessarily have a parent_id in their data source or are tracks with no
concept of a stack or depth.

This patch relaxes the parent_id constraint, allowing tracks both with
and without parent_id to be aggregated.

Fixes https://buganizer.corp.google.com/issues/448068359
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.

2 participants