[Bug/Enhancement] Sched plugin aggregates CPU slices by process_name, not PID, impacting analysis (v54)

[wenyongli1122]

Hello!

I've been using Perfetto for trace analysis recently and it's a fantastic tool. While exploring the UI, I've noticed an aggregation behavior in the cpu_slice_by_process_selection_aggregator.ts plugin that I believe could be improved for more precise analysis.

It looks like the plugin, specifically observed in v54, aggregates CPU slices based solely on process_name. This can be problematic, especially when dealing with systrace data where multiple processes might share the same name but have distinct Process IDs (PIDs). For example, if you have several instances of a shell or a service that forks child processes with similar names, all their CPU slices get grouped together under one process_name entry.

This aggregation method leads to several issues:

• Misleading Data: Users might misinterpret aggregated data as representing a single process, when in reality it's a mix of several.
• Difficulty in Debugging: It becomes challenging to pinpoint performance issues or resource consumption for a specific process instance when its data is merged with others.
• Reduced Granularity: The inability to analyze by PID reduces the granularity and effectiveness of the Sched plugin for certain complex tracing scenarios.

My initial thought is that for more precise and meaningful analysis, the CPU slice aggregation should ideally consider process_pid in addition to (or as the primary key for) process_name. This would allow users to differentiate between distinct process instances even if they share the same name.

I haven't delved deep into the UI's plugin architecture yet, but I imagine a solution could involve:

• Prioritizing process_pid for aggregation, perhaps by creating a unique key like ${process_name} [${process_pid}].
• Providing an option in the UI to switch between process_name and process_pid based aggregation.
• By default, aggregating by process_name but allowing expanding to show individual process_pid entries.

If this is something other folks are interested in or if there's a current workaround I'm missing, I'd be happy to give it a shot at exploring potential solutions or contributing. Wanted to put feelers out before typing up a full proposal.

Thank you for your time and consideration!

Location of the issue:
ui/src/plugins/dev.perfetto.Sched/cpu_slice_by_process_selection_aggregator.ts

Version Affected:
Perfetto UI v54 (and potentially earlier/later versions if the logic hasn't changed).

Steps to Reproduce (Conceptual):

1. Record a systrace or any trace where multiple processes have the same process_name but different PIDs (e.g., multiple instances of a shell, or a service that forks child processes with similar names).
2. Load the recorded trace in Perfetto UI v54.
3. Navigate to the Sched slice table or any view that utilizes the cpu_slice_by_process_selection_aggregator.
4. Observe that CPU slices belonging to different PIDs but the same process_name are grouped together, without an option to distinguish by PID.

[Vikx001]

Hi, I've submitted a fix for this in #5229

Root cause: The groupBy array in getColumnDefinitions() for both
cpu_slice_by_process_selection_aggregator and cpu_slice_selection_aggregator
only used process_name/thread_name as the grouping key. This meant the
underlying SQL grouped by a single column (GROUP BY process_name), causing
two processes with the same name but different PIDs to be merged into one row.

Fix: Added pid to the groupBy for the "CPU by process" panel and pid +
tid for the "CPU by thread" panel. Both were already present in the SQL SELECT
and already displayed as columns — only the GROUP BY key was missing them.

please lemme know if anything else can be done or expected.

[LalitMaganti]

@stevegolton for thoughts

[stevegolton]

Thanks for raising the issue. The correct field to group on would be upid, which is guaranteed to be unique within the trace. The problem is upids are not particularly readable.

We could do one of the following:

• Group by both upid and process name - awkward, especially in tree mode.
• Group by upid only but add the process name as an 'ANY' aggregate on the RHS. Awkward - some distance between upid and process name. Any
• Create a custom composite field in the original table, something like (${upid}) ${process.name}. Not ideal as still grouping on a string rather than an indexed number but no worse than what we have now. Does involve string building in SQL which can be slow for large datasets.
• Create a new concept in DataGrid for a 'compound field' which uses upid as its underlying value and styles the fields dynamically at runtime from the upid and process.name cols. I actually had this implemented a while back but it was awkward for various reasons (sorting didn't work as expected, loading distinct values were awkward).

I'm not sure what the best answer is here.

[stevegolton]

OK, after discussing a bit internally, I'm going to revise my comment a little. There are valid reasons for wanting group by process name only. Consider a daemon/cron job that starts a process periodically which does some work then shuts down. Every invocation would have a unique pid and upid, but normally you'd probably want to batch all these process invocations up in one bucket when aggregating.

We suspect this is actually the more common case, so I think we're going to leave this as the default for now.

You can always add pid as an extra group by column manually.