Skip to content

Commit

Permalink
ui: simplify handling of AreaSelection
Browse files Browse the repository at this point in the history
Today Area selection contains a list of trackUris,
and then every selection aggregator ends up resolving them
by calling trackManager.getTrack().
Instead this CL does the resolution in selectionManager, before
setting the selection object. This allows getting rid to various
calls to globals.trackManager in the selection aggregators.
In turn this allows moving the wattson plugin from
core_plugins -> plugins, as now there is nothing special with
it anymore.
Also remove recording of action args in metatracing. Nobody is
using it (especially now that Actions is disappearing) and causes
noticeable jank with this change because I'm pushing the track
descriptors into AreaSelection, and in turn the PivotTableController
passes the area selection as an action argument. So we end up
calling metatracing.flattenArgs on all the resolved tracks for no
reason

Change-Id: I0c7f77d162c419aeb82fb69f4074a4e9f1d21613
  • Loading branch information
primiano committed Sep 26, 2024
1 parent 0cc664e commit 53d2aa9
Show file tree
Hide file tree
Showing 19 changed files with 84 additions and 88 deletions.
3 changes: 1 addition & 2 deletions ui/src/controller/flow_events_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -360,8 +360,7 @@ export class FlowEventsController extends Controller<'main'> {
private areaSelected(area: AreaSelection) {
const trackIds: number[] = [];

for (const trackUri of area.trackUris) {
const trackInfo = globals.trackManager.getTrack(trackUri);
for (const trackInfo of area.tracks) {
const kind = trackInfo?.tags?.kind;
if (
kind === THREAD_SLICE_TRACK_KIND ||
Expand Down
8 changes: 4 additions & 4 deletions ui/src/core/selection_aggregation_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {AsyncLimiter} from '../base/async_limiter';
import {isString} from '../base/object_utils';
import {Optional} from '../base/utils';
import {AggregateData, Column, ColumnDef, Sorting} from '../public/aggregation';
import {Area, AreaSelectionAggregator} from '../public/selection';
import {AreaSelection, AreaSelectionAggregator} from '../public/selection';
import {Engine} from '../trace_processor/engine';
import {NUM} from '../trace_processor/query_result';
import {raf} from './raf_scheduler';
Expand All @@ -27,7 +27,7 @@ export class SelectionAggregationManager {
private _aggregators = new Array<AreaSelectionAggregator>();
private _aggregatedData = new Map<string, AggregateData>();
private _sorting = new Map<string, Sorting>();
private _currentArea: Optional<Area> = undefined;
private _currentArea: Optional<AreaSelection> = undefined;

constructor(engine: Engine) {
this.engine = engine;
Expand All @@ -37,7 +37,7 @@ export class SelectionAggregationManager {
this._aggregators.push(aggr);
}

aggregateArea(area: Area) {
aggregateArea(area: AreaSelection) {
this.limiter.schedule(async () => {
this._currentArea = area;
this._aggregatedData.clear();
Expand Down Expand Up @@ -103,7 +103,7 @@ export class SelectionAggregationManager {

private async runAggregator(
aggr: AreaSelectionAggregator,
area: Area,
area: AreaSelection,
): Promise<AggregateData | undefined> {
const viewExists = await aggr.createAggregateView(this.engine, area);
if (!viewExists) {
Expand Down
19 changes: 17 additions & 2 deletions ui/src/core/selection_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export class SelectionManagerImpl implements SelectionManager {
assertTrue(start <= end);
this.setSelection({
kind: 'area',
tracks: [],
...args,
});
}
Expand Down Expand Up @@ -302,14 +303,28 @@ export class SelectionManagerImpl implements SelectionManager {
}

private setSelection(selection: Selection, opts?: SelectionOpts) {
if (selection.kind === 'area') {
// In the case of area selection, the caller provides a list of trackUris.
// However, all the consumer want to access the resolved TrackDescriptor.
// Rather than delegating this to the various consumers, we resolve them
// now once and for all and place them in the selection object.
const tracks = [];
for (const uri of selection.trackUris) {
const trackDescr = this.trackManager.getTrack(uri);
if (trackDescr === undefined) continue;
tracks.push(trackDescr);
}
selection = {...selection, tracks};
}

this._selection = selection;
this._pendingScrollId = opts?.pendingScrollId;
this.onSelectionChange(selection, opts ?? {});
const generation = ++this._selectionGeneration;
raf.scheduleFullRedraw();

if (this.selection.kind === 'area') {
this._aggregationManager.aggregateArea(this.selection);
if (this._selection.kind === 'area') {
this._aggregationManager.aggregateArea(this._selection);
} else {
this._aggregationManager.clear();
}
Expand Down
8 changes: 3 additions & 5 deletions ui/src/core_plugins/counter/counter_selection_aggregator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,17 @@

import {Duration} from '../../base/time';
import {ColumnDef, Sorting} from '../../public/aggregation';
import {Area} from '../../public/selection';
import {globals} from '../../frontend/globals';
import {AreaSelection} from '../../public/selection';
import {COUNTER_TRACK_KIND} from '../../public/track_kinds';
import {Engine} from '../../trace_processor/engine';
import {AreaSelectionAggregator} from '../../public/selection';

export class CounterSelectionAggregator implements AreaSelectionAggregator {
readonly id = 'counter_aggregation';

async createAggregateView(engine: Engine, area: Area) {
async createAggregateView(engine: Engine, area: AreaSelection) {
const trackIds: (string | number)[] = [];
for (const trackUri of area.trackUris) {
const trackInfo = globals.trackManager.getTrack(trackUri);
for (const trackInfo of area.tracks) {
if (trackInfo?.tags?.kind === COUNTER_TRACK_KIND) {
trackInfo.tags?.trackIds && trackIds.push(...trackInfo.tags.trackIds);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@

import {exists} from '../../base/utils';
import {ColumnDef, Sorting} from '../../public/aggregation';
import {Area} from '../../public/selection';
import {globals} from '../../frontend/globals';
import {AreaSelection} from '../../public/selection';
import {Engine} from '../../trace_processor/engine';
import {CPU_SLICE_TRACK_KIND} from '../../public/track_kinds';
import {AreaSelectionAggregator} from '../../public/selection';
Expand All @@ -25,10 +24,9 @@ export class CpuSliceByProcessSelectionAggregator
{
readonly id = 'cpu_by_process_aggregation';

async createAggregateView(engine: Engine, area: Area) {
async createAggregateView(engine: Engine, area: AreaSelection) {
const selectedCpus: number[] = [];
for (const trackUri of area.trackUris) {
const trackInfo = globals.trackManager.getTrack(trackUri);
for (const trackInfo of area.tracks) {
if (trackInfo?.tags?.kind === CPU_SLICE_TRACK_KIND) {
exists(trackInfo.tags.cpu) && selectedCpus.push(trackInfo.tags.cpu);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,17 @@

import {exists} from '../../base/utils';
import {ColumnDef, Sorting} from '../../public/aggregation';
import {Area} from '../../public/selection';
import {AreaSelection} from '../../public/selection';
import {CPU_SLICE_TRACK_KIND} from '../../public/track_kinds';
import {globals} from '../../frontend/globals';
import {Engine} from '../../trace_processor/engine';
import {AreaSelectionAggregator} from '../../public/selection';

export class CpuSliceSelectionAggregator implements AreaSelectionAggregator {
readonly id = 'cpu_aggregation';

async createAggregateView(engine: Engine, area: Area) {
async createAggregateView(engine: Engine, area: AreaSelection) {
const selectedCpus: number[] = [];
for (const trackUri of area.trackUris) {
const trackInfo = globals.trackManager.getTrack(trackUri);
for (const trackInfo of area.tracks) {
if (trackInfo?.tags?.kind === CPU_SLICE_TRACK_KIND) {
exists(trackInfo.tags.cpu) && selectedCpus.push(trackInfo.tags.cpu);
}
Expand Down
3 changes: 1 addition & 2 deletions ui/src/core_plugins/critical_path/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ const sliceColumnNames = ['id', 'utid', 'ts', 'dur', 'name', 'table_name'];
function getFirstUtidOfSelectionOrVisibleWindow(trace: Trace): number {
const selection = trace.selection.selection;
if (selection.kind === 'area') {
for (const trackUri of selection.trackUris) {
const trackDesc = trace.tracks.getTrack(trackUri);
for (const trackDesc of selection.tracks) {
if (
trackDesc?.tags?.kind === THREAD_STATE_TRACK_KIND &&
trackDesc?.tags?.utid !== undefined
Expand Down
8 changes: 3 additions & 5 deletions ui/src/core_plugins/frames/frame_selection_aggregator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,17 @@
// limitations under the License.

import {ColumnDef, Sorting} from '../../public/aggregation';
import {Area} from '../../public/selection';
import {AreaSelection} from '../../public/selection';
import {ACTUAL_FRAMES_SLICE_TRACK_KIND} from '../../public/track_kinds';
import {globals} from '../../frontend/globals';
import {Engine} from '../../trace_processor/engine';
import {AreaSelectionAggregator} from '../../public/selection';

export class FrameSelectionAggregator implements AreaSelectionAggregator {
readonly id = 'frame_aggregation';

async createAggregateView(engine: Engine, area: Area) {
async createAggregateView(engine: Engine, area: AreaSelection) {
const selectedSqlTrackIds: number[] = [];
for (const trackUri of area.trackUris) {
const trackInfo = globals.trackManager.getTrack(trackUri);
for (const trackInfo of area.tracks) {
if (trackInfo?.tags?.kind === ACTUAL_FRAMES_SLICE_TRACK_KIND) {
trackInfo.tags.trackIds &&
selectedSqlTrackIds.push(...trackInfo.tags.trackIds);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@
// limitations under the License.

import {ColumnDef, Sorting} from '../../public/aggregation';
import {Area} from '../../public/selection';
import {globals} from '../../frontend/globals';
import {AreaSelection} from '../../public/selection';
import {Engine} from '../../trace_processor/engine';
import {AreaSelectionAggregator} from '../../public/selection';
import {
Expand All @@ -27,7 +26,7 @@ export class AsyncAndThreadSliceSelectionAggregator
{
readonly id = 'slice_aggregation';

async createAggregateView(engine: Engine, area: Area) {
async createAggregateView(engine: Engine, area: AreaSelection) {
const selectedTrackKeys = getSelectedTrackSqlIds(area);

if (selectedTrackKeys.length === 0) return false;
Expand Down Expand Up @@ -90,10 +89,9 @@ export class AsyncAndThreadSliceSelectionAggregator
}
}

function getSelectedTrackSqlIds(area: Area): number[] {
function getSelectedTrackSqlIds(area: AreaSelection): number[] {
const selectedTrackKeys: number[] = [];
for (const trackUri of area.trackUris) {
const trackInfo = globals.trackManager.getTrack(trackUri);
for (const trackInfo of area.tracks) {
if (trackInfo?.tags?.kind === THREAD_SLICE_TRACK_KIND) {
trackInfo.tags.trackIds &&
selectedTrackKeys.push(...trackInfo.tags.trackIds);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,30 +14,29 @@

import {exists} from '../../base/utils';
import {ColumnDef, Sorting, ThreadStateExtra} from '../../public/aggregation';
import {Area} from '../../public/selection';
import {AreaSelection} from '../../public/selection';
import {THREAD_STATE_TRACK_KIND} from '../../public/track_kinds';
import {globals} from '../../frontend/globals';
import {Engine} from '../../trace_processor/engine';
import {NUM, NUM_NULL, STR_NULL} from '../../trace_processor/query_result';
import {AreaSelectionAggregator} from '../../public/selection';
import {translateState} from '../../trace_processor/sql_utils/thread_state';
import {TrackDescriptor} from '../../public/track';

export class ThreadStateSelectionAggregator implements AreaSelectionAggregator {
readonly id = 'thread_state_aggregation';
private utids?: number[];

setThreadStateUtids(tracks: ReadonlyArray<string>) {
setThreadStateUtids(tracks: ReadonlyArray<TrackDescriptor>) {
this.utids = [];
for (const trackUri of tracks) {
const trackInfo = globals.trackManager.getTrack(trackUri);
for (const trackInfo of tracks) {
if (trackInfo?.tags?.kind === THREAD_STATE_TRACK_KIND) {
exists(trackInfo.tags.utid) && this.utids.push(trackInfo.tags.utid);
}
}
}

async createAggregateView(engine: Engine, area: Area) {
this.setThreadStateUtids(area.trackUris);
async createAggregateView(engine: Engine, area: AreaSelection) {
this.setThreadStateUtids(area.tracks);
if (this.utids === undefined || this.utids.length === 0) return false;

await engine.query(`
Expand All @@ -63,8 +62,11 @@ export class ThreadStateSelectionAggregator implements AreaSelectionAggregator {
return true;
}

async getExtra(engine: Engine, area: Area): Promise<ThreadStateExtra | void> {
this.setThreadStateUtids(area.trackUris);
async getExtra(
engine: Engine,
area: AreaSelection,
): Promise<ThreadStateExtra | void> {
this.setThreadStateUtids(area.tracks);
if (this.utids === undefined || this.utids.length === 0) return;

const query = `
Expand Down
12 changes: 4 additions & 8 deletions ui/src/frontend/aggregation_tab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,7 @@ class AreaDetailsPanel implements m.ClassComponent<AreaDetailsPanelAttrs> {
return this.cpuProfileFlamegraphAttrs;
}
const utids = [];
for (const trackUri of currentSelection.trackUris) {
const trackInfo = globals.trackManager.getTrack(trackUri);
for (const trackInfo of currentSelection.tracks) {
if (trackInfo?.tags?.kind === CPU_PROFILE_TRACK_KIND) {
utids.push(trackInfo.tags?.utid);
}
Expand Down Expand Up @@ -321,8 +320,7 @@ class AreaDetailsPanel implements m.ClassComponent<AreaDetailsPanelAttrs> {
return this.sliceFlamegraphAttrs;
}
const trackIds = [];
for (const trackUri of currentSelection.trackUris) {
const trackInfo = globals.trackManager.getTrack(trackUri);
for (const trackInfo of currentSelection.tracks) {
if (trackInfo?.tags?.kind !== THREAD_SLICE_TRACK_KIND) {
continue;
}
Expand Down Expand Up @@ -394,8 +392,7 @@ export class AggregationsTabs implements Disposable {

function getUpidsFromPerfSampleAreaSelection(currentSelection: AreaSelection) {
const upids = [];
for (const trackUri of currentSelection.trackUris) {
const trackInfo = globals.trackManager.getTrack(trackUri);
for (const trackInfo of currentSelection.tracks) {
if (
trackInfo?.tags?.kind === PERF_SAMPLES_PROFILE_TRACK_KIND &&
trackInfo.tags?.utid === undefined
Expand All @@ -408,8 +405,7 @@ function getUpidsFromPerfSampleAreaSelection(currentSelection: AreaSelection) {

function getUtidsFromPerfSampleAreaSelection(currentSelection: AreaSelection) {
const utids = [];
for (const trackUri of currentSelection.trackUris) {
const trackInfo = globals.trackManager.getTrack(trackUri);
for (const trackInfo of currentSelection.tracks) {
if (
trackInfo?.tags?.kind === PERF_SAMPLES_PROFILE_TRACK_KIND &&
trackInfo.tags?.utid !== undefined
Expand Down
20 changes: 7 additions & 13 deletions ui/src/frontend/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {defer} from '../base/deferred';
import {addErrorHandler, reportError} from '../base/logging';
import {Store} from '../base/store';
import {Actions, DeferredAction, StateActions} from '../common/actions';
import {flattenArgs, traceEvent} from '../common/metatracing';
import {traceEvent} from '../common/metatracing';
import {pluginManager} from '../common/plugins';
import {State} from '../common/state';
import {initController, runControllers} from '../controller';
Expand Down Expand Up @@ -412,18 +412,12 @@ function maybeChangeRpcPortFromFragment() {

function stateActionDispatcher(actions: DeferredAction[]) {
const edits = actions.map((action) => {
return traceEvent(
`action.${action.type}`,
() => {
return (draft: Draft<State>) => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(StateActions as any)[action.type](draft, action.args);
};
},
{
args: flattenArgs(action.args),
},
);
return traceEvent(`action.${action.type}`, () => {
return (draft: Draft<State>) => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(StateActions as any)[action.type](draft, action.args);
};
});
});
globals.store.edit(edits);
}
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@
// limitations under the License.

import {ColumnDef, Sorting} from '../../public/aggregation';
import {Area} from '../../public/selection';
import {globals} from '../../frontend/globals';
import {Area, AreaSelection} from '../../public/selection';
import {Engine} from '../../trace_processor/engine';
import {CPUSS_ESTIMATE_TRACK_KIND} from '../../public/track_kinds';
import {AreaSelectionAggregator} from '../../public/selection';
Expand All @@ -25,12 +24,11 @@ export class WattsonEstimateSelectionAggregator
{
readonly id = 'wattson_estimate_aggregation';

async createAggregateView(engine: Engine, area: Area) {
async createAggregateView(engine: Engine, area: AreaSelection) {
await engine.query(`drop view if exists ${this.id};`);

const estimateTracks: string[] = [];
for (const trackUri of area.trackUris) {
const trackInfo = globals.trackManager.getTrack(trackUri);
for (const trackInfo of area.tracks) {
if (
trackInfo?.tags?.kind === CPUSS_ESTIMATE_TRACK_KIND &&
exists(trackInfo.tags?.wattson)
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@

import {exists} from '../../base/utils';
import {ColumnDef, Sorting} from '../../public/aggregation';
import {Area} from '../../public/selection';
import {globals} from '../../frontend/globals';
import {AreaSelection} from '../../public/selection';
import {Engine} from '../../trace_processor/engine';
import {NUM} from '../../trace_processor/query_result';
import {CPU_SLICE_TRACK_KIND} from '../../public/track_kinds';
Expand All @@ -26,7 +25,7 @@ export class WattsonPackageSelectionAggregator
{
readonly id = 'wattson_package_aggregation';

async createAggregateView(engine: Engine, area: Area) {
async createAggregateView(engine: Engine, area: AreaSelection) {
await engine.query(`drop view if exists ${this.id};`);

const packageInfo = await engine.query(`
Expand All @@ -37,8 +36,7 @@ export class WattsonPackageSelectionAggregator
if (packageInfo.firstRow({isValid: NUM}).isValid === 0) return false;

const selectedCpus: number[] = [];
for (const trackUri of area.trackUris) {
const trackInfo = globals.trackManager.getTrack(trackUri);
for (const trackInfo of area.tracks) {
if (trackInfo?.tags?.kind === CPU_SLICE_TRACK_KIND) {
exists(trackInfo.tags.cpu) && selectedCpus.push(trackInfo.tags.cpu);
}
Expand Down
Loading

0 comments on commit 53d2aa9

Please sign in to comment.