Add slow start insights track with slices of interest #1155
Conversation
Change-Id: Ieb1ccae6e8a5763f6a08839e168a0166e37ec480
| let slice_glob = `'OpenDexFilesFromOat*'`; | ||
| let result = await trace.engine.query( | ||
| ` | ||
| SELECT RUN_METRIC('android/startup/slow_start_thresholds.sql'); |
There was a problem hiding this comment.
Using RUN_METRIC in plugins is not allowed. There should be a presubmit check warning you about this if you did tools/install-build-deps. Please port all relevant things you need to the standard library first before trying to use them here.
There was a problem hiding this comment.
This applies here and for all other usages of RUN_METRIC in this class.
LalitMaganti
left a comment
LalitMaganti
left a comment
There was a problem hiding this comment.
Requesting changes as above.
| * @returns a track node with the slow start status. | ||
| * `undefined` if there are no app startups detected. | ||
| */ | ||
| export async function slowInsightsTrack( |
There was a problem hiding this comment.
If you already have the data already available in a table/view, you should just create the track directly from that by crafting the correct SQL query instead of pulling it all out and injecting it back in again.
I.e.
const track = await createQuerySliceTrack({
trace: trace,
uri: uri,
data: {
sqlSource: `
SELECT
startup_id AS id,
ts,
ts_end - ts as dur
FROM android_startups
`,
columns: ['ts', 'dur', 'name'],
},
});
You only need to inject the data if you don't have it available in trace processor already.
| const startups: Array<Startup> = []; | ||
|
|
||
| // Find app startups | ||
| let result = await trace.engine.query( |
There was a problem hiding this comment.
I think most if not all of these lets can be consts. Eslint should complain if lets are not reassigned.
Our presubmit checks are still up in the air since the move to github, but you should be able to run eslint with ui/format-sources.
| ): Promise<TrackNode | undefined> { | ||
| // The log tag | ||
| let tag = 'Broadcast received'; | ||
| let slice_glob = `'broadcastReceiveReg*'`; |
There was a problem hiding this comment.
Variables should be in snakeCase. There's quite a few instances of this here.
| return getSlowReasonTrack(trace, slices, uri, title); | ||
| } | ||
|
|
||
| async function getBindApplicationTrack( |
There was a problem hiding this comment.
In general there's a lot of duplicated code in this file that makes it very difficult to understand exactly what you're trying to do. Could you remove some of this duplication please?
3 participants
Change-Id: Ieb1ccae6e8a5763f6a08839e168a0166e37ec480
Bug: 376308737
Test: ui/run-dev-server with ptraces
UI snapshot: https://screenshot.googleplex.com/6Fqk5FHFtDJKnwb