Skip to content

Commit

Permalink
Decouple CUJ pinning from JankCUJ metrics
Browse files Browse the repository at this point in the history
This allows to prevent pinning the same CUJ multiple times if there are multiple metrics related to it.

Also, each metric could be matched by multiple handlers now.

Bug: 354119654
Test: opened perfetto with 2 metrics to pin related to the same CUJ. Verified the CUJ is pinned once
Change-Id: I26ac544ce78ebb7a6e3734f356e264bcf9d32caf
  • Loading branch information
nicomazz committed Aug 12, 2024
1 parent 900e4c4 commit 446975a
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ import {MetricHandler} from './metricUtils';
import {pinBlockingCallHandlerInstance} from './pinBlockingCall';
import {pinCujScopedJankInstance} from './pinCujScoped';
import {pinFullTraceJankInstance} from './fullTraceJankMetricHandler';
import {pinCujInstance} from './pinCujMetricHandler';

// TODO: b/337774166 - Add handlers for the metric name categories here
export const METRIC_HANDLERS: MetricHandler[] = [
pinCujInstance,
pinCujScopedJankInstance,
pinBlockingCallHandlerInstance,
pinFullTraceJankInstance,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,17 @@ export interface BlockingCallMetricData {
aggregation: string;
}

/** Represents a cuj to be pinned. */
export interface CujMetricData {
cujName: string;
}

// Common MetricData for all handler. If new needed then add here.
export type MetricData =
| FullTraceMetricData
| CujScopedMetricData
| BlockingCallMetricData;
| BlockingCallMetricData
| CujMetricData;

// Common JankType for cujScoped and fullTrace metrics
export type JankType = 'sf_frames' | 'app_frames' | 'frames';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Copyright (C) 2024 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import {CujMetricData, MetricHandler} from './metricUtils';
import {PluginContextTrace} from '../../../public';
import {addJankCUJDebugTrack} from '../../dev.perfetto.AndroidCujs';
import {TrackType} from '../../dev.perfetto.AndroidCujs/trackUtils';
import {PLUGIN_ID} from '../pluginId';

/** Pins a single CUJ from CUJ scoped metrics. */
class PinCujMetricHandler implements MetricHandler {
/**
* Matches metric key & return parsed data if successful.
*
* @param {string} metricKey The metric key to match.
* @returns {CujMetricData | undefined} Parsed data or undefined if no match.
*/
public match(metricKey: string): CujMetricData | undefined {
const matcher = /perfetto_cuj_(?<process>.*)-(?<cujName>.*)-.*-missed_.*/;
const match = matcher.exec(metricKey);
if (!match?.groups) {
return undefined;
}
return {
cujName: match.groups.cujName,
};
}

/**
* Adds the debug tracks for cuj Scoped jank metrics
* registerStaticTrack used when plugin adds tracks onTraceload()
* addDebugSliceTrack used for adding tracks using the command
*
* @param {CujMetricData} metricData Parsed metric data for the cuj scoped jank
* @param {PluginContextTrace} ctx PluginContextTrace for trace related properties and methods
* @param {TrackType} type 'static' for onTraceload and 'debug' for command
* @returns {void} Adds one track for Jank CUJ slice and one for Janky CUJ frames
*/
public async addMetricTrack(
metricData: CujMetricData,
ctx: PluginContextTrace,
type: TrackType,
) {
this.pinSingleCuj(ctx, metricData.cujName, type);
}

private pinSingleCuj(
ctx: PluginContextTrace,
cujName: string,
type: TrackType,
) {
const uri = `${PLUGIN_ID}#PinCuj#${cujName}`;
const trackName = `Jank CUJ: ${cujName}`;
addJankCUJDebugTrack(ctx, trackName, type, cujName, uri);
}
}

export const pinCujInstance = new PinCujMetricHandler();
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {
import {LONG, NUM} from '../../../trace_processor/query_result';
import {PluginContextTrace} from '../../../public';
import {SimpleSliceTrackConfig} from '../../../frontend/simple_slice_track';
import {addJankCUJDebugTrack} from '../../dev.perfetto.AndroidCujs';
import {
addAndPinSliceTrack,
focusOnSlice,
Expand Down Expand Up @@ -73,7 +72,6 @@ class PinCujScopedJank implements MetricHandler {
// TODO: b/349502258 - Refactor to single API
const {config: cujScopedJankSlice, trackName: trackName} =
await this.cujScopedTrackConfig(metricData, ctx);
this.pinSingleCuj(ctx, metricData, type);
const uri = `${PLUGIN_ID}#CUJScopedJankSlice#${metricData}`;

addAndPinSliceTrack(ctx, cujScopedJankSlice, trackName, type, uri);
Expand All @@ -82,16 +80,6 @@ class PinCujScopedJank implements MetricHandler {
}
}

private pinSingleCuj(
ctx: PluginContextTrace,
metricData: CujScopedMetricData,
type: TrackType,
) {
const uri = `${PLUGIN_ID}#CUJScopedBoundaryTimes#${metricData}`;
const trackName = `Jank CUJ: ${metricData.cujName}`;
addJankCUJDebugTrack(ctx, trackName, type, metricData.cujName, uri);
}

private async cujScopedTrackConfig(
metricData: CujScopedMetricData,
ctx: PluginContextTrace,
Expand All @@ -112,7 +100,7 @@ class PinCujScopedJank implements MetricHandler {
const processName = metricData.process;

const createJankyCujFrameTable = `
CREATE PERFETTO TABLE _janky_frames_during_cuj_from_metric_key AS
CREATE OR REPLACE PERFETTO TABLE _janky_frames_during_cuj_from_metric_key AS
SELECT
f.vsync as id,
f.ts AS ts,
Expand Down
38 changes: 20 additions & 18 deletions ui/src/plugins/dev.perfetto.PinAndroidPerfMetrics/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import {Plugin, PluginContextTrace, PluginDescriptor} from '../../public';
import {TrackType} from '../dev.perfetto.AndroidCujs/trackUtils';
import {METRIC_HANDLERS} from './handlers/handlerRegistry';
import {MetricHandlerMatch} from './handlers/metricUtils';
import {MetricData, MetricHandlerMatch} from './handlers/metricUtils';
import {PLUGIN_ID} from './pluginId';

const JANK_CUJ_QUERY_PRECONDITIONS = `
Expand Down Expand Up @@ -91,28 +91,30 @@ class PinAndroidPerfMetrics implements Plugin {
return metricList.map((metric) => decodeURIComponent(metric));
}

private getMetricsToShow(metricList: string[]) {
private getMetricsToShow(metricList: string[]): MetricHandlerMatch[] {
const sortedMetricList = [...metricList].sort();
const validMetrics: MetricHandlerMatch[] = [];
metricList.forEach((metric) => {
const matchedHandler = this.matchMetricToHandler(metric);
if (matchedHandler) {
validMetrics.push(matchedHandler);
const alreadyMatchedMetricData: Set<string> = new Set();
for (const metric of sortedMetricList) {
for (const metricHandler of METRIC_HANDLERS) {
const metricData = metricHandler.match(metric);
if (!metricData) continue;
const jsonMetricData = this.metricDataToJson(metricData);
if (!alreadyMatchedMetricData.has(jsonMetricData)) {
alreadyMatchedMetricData.add(jsonMetricData);
validMetrics.push({
metricData: metricData,
metricHandler: metricHandler,
});
}
}
});
}
return validMetrics;
}

private matchMetricToHandler(metric: string): MetricHandlerMatch | null {
for (const metricHandler of METRIC_HANDLERS) {
const match = metricHandler.match(metric);
if (match) {
return {
metricData: match,
metricHandler: metricHandler,
};
}
}
return null;
private metricDataToJson(metricData: MetricData): string {
// Used to have a deterministic keys order.
return JSON.stringify(metricData, Object.keys(metricData).sort());
}
}

Expand Down

0 comments on commit 446975a

Please sign in to comment.