Skip to content

Commit

Permalink
ui: Remove some more gratuitous use of globals in tracks/timeline
Browse files Browse the repository at this point in the history
Change-Id: I2f45543e6555918a12b4d6304af0e573bc74e9d1
  • Loading branch information
stevegolton committed Oct 17, 2024
1 parent e0e9166 commit 7dfb6b5
Show file tree
Hide file tree
Showing 14 changed files with 337 additions and 313 deletions.
14 changes: 0 additions & 14 deletions ui/src/common/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
// limitations under the License.

import {Draft} from 'immer';
import {time} from '../base/time';
import {RecordConfig} from '../controller/record_config_types';
import {createEmptyState} from './empty_state';
import {
Expand Down Expand Up @@ -128,19 +127,6 @@ export const StateActions = {
state.sidebarVisible = args.visible;
},

setHoveredUtidAndPid(state: StateDraft, args: {utid: number; pid: number}) {
state.hoveredPid = args.pid;
state.hoveredUtid = args.utid;
},

setHighlightedSliceId(state: StateDraft, args: {sliceId: number}) {
state.highlightedSliceId = args.sliceId;
},

setHoveredNoteTimestamp(state: StateDraft, args: {ts: time}) {
state.hoveredNoteTimestamp = args.ts;
},

dismissFlamegraphModal(state: StateDraft, _: {}) {
state.flamegraphModalDismissed = true;
},
Expand Down
5 changes: 0 additions & 5 deletions ui/src/common/empty_state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import {Time} from '../base/time';
import {createEmptyRecordConfig} from '../controller/record_config_types';
import {featureFlags} from '../core/feature_flags';
import {
Expand Down Expand Up @@ -55,10 +54,6 @@ export function createEmptyState(): State {

perfDebug: false,
sidebarVisible: true,
hoveredUtid: -1,
hoveredPid: -1,
hoveredNoteTimestamp: Time.INVALID,
highlightedSliceId: -1,

recordingInProgress: false,
recordingCancelled: false,
Expand Down
7 changes: 0 additions & 7 deletions ui/src/common/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import {time} from '../base/time';
import {RecordConfig} from '../controller/record_config_types';
import {TraceSource} from '../public/trace_source';

Expand Down Expand Up @@ -181,12 +180,6 @@ export interface State {
// Show the sidebar extended
sidebarVisible: boolean;

// Hovered and focused events
hoveredUtid: number;
hoveredPid: number;
hoveredNoteTimestamp: time;
highlightedSliceId: number;

/**
* Trace recording
*/
Expand Down
44 changes: 44 additions & 0 deletions ui/src/core/timeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,50 @@ const MIN_DURATION = 10;
export class TimelineImpl implements Timeline {
private _visibleWindow: HighPrecisionTimeSpan;
private _hoverCursorTimestamp?: time;
private _highlightedSliceId?: number;
private _hoveredNoteTimestamp?: time;

// TODO(stevegolton): These are currently only referenced by the cpu slice
// tracks and the process summary tracks. We should just make this a local
// property of the cpu slice tracks and ignore them in the process tracks.
private _hoveredUtid?: number;
private _hoveredPid?: number;

get highlightedSliceId() {
return this._highlightedSliceId;
}

set highlightedSliceId(x) {
this._highlightedSliceId = x;
raf.scheduleFullRedraw();
}

get hoveredNoteTimestamp() {
return this._hoveredNoteTimestamp;
}

set hoveredNoteTimestamp(x) {
this._hoveredNoteTimestamp = x;
raf.scheduleFullRedraw();
}

get hoveredUtid() {
return this._hoveredUtid;
}

set hoveredUtid(x) {
this._hoveredUtid = x;
raf.scheduleFullRedraw();
}

get hoveredPid() {
return this._hoveredPid;
}

set hoveredPid(x) {
this._hoveredPid = x;
raf.scheduleFullRedraw();
}

// This is used to calculate the tracks within a Y range for area selection.
areaY: Range = {};
Expand Down
31 changes: 14 additions & 17 deletions ui/src/core_plugins/cpu_freq/cpu_freq_track.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,14 @@ import {colorForCpu} from '../../core/colorizer';
import {TrackData} from '../../common/track_data';
import {TimelineFetcher} from '../../common/track_helper';
import {checkerboardExcept} from '../../frontend/checkerboard';
import {globals} from '../../frontend/globals';
import {Engine} from '../../trace_processor/engine';
import {Track} from '../../public/track';
import {LONG, NUM} from '../../trace_processor/query_result';
import {uuidv4Sql} from '../../base/uuid';
import {TrackMouseEvent, TrackRenderContext} from '../../public/track';
import {Point2D} from '../../base/geom';
import {createView, createVirtualTable} from '../../trace_processor/sql_utils';
import {AsyncDisposableStack} from '../../base/disposable_stack';
import {Trace} from '../../public/trace';

export interface Data extends TrackData {
timestamps: BigInt64Array;
Expand Down Expand Up @@ -58,23 +57,21 @@ export class CpuFreqTrack implements Track {
private hoveredIdle: number | undefined = undefined;
private fetcher = new TimelineFetcher<Data>(this.onBoundsChange.bind(this));

private engine: Engine;
private config: Config;
private trackUuid = uuidv4Sql();

private trash!: AsyncDisposableStack;

constructor(config: Config, engine: Engine) {
this.config = config;
this.engine = engine;
}
constructor(
private readonly config: Config,
private readonly trace: Trace,
) {}

async onCreate() {
this.trash = new AsyncDisposableStack();
if (this.config.idleTrackId === undefined) {
this.trash.use(
await createView(
this.engine,
this.trace.engine,
`raw_freq_idle_${this.trackUuid}`,
`
select ts, dur, value as freqValue, -1 as idleValue
Expand All @@ -86,7 +83,7 @@ export class CpuFreqTrack implements Track {
} else {
this.trash.use(
await createView(
this.engine,
this.trace.engine,
`raw_freq_${this.trackUuid}`,
`
select ts, dur, value as freqValue
Expand All @@ -98,7 +95,7 @@ export class CpuFreqTrack implements Track {

this.trash.use(
await createView(
this.engine,
this.trace.engine,
`raw_idle_${this.trackUuid}`,
`
select
Expand All @@ -113,7 +110,7 @@ export class CpuFreqTrack implements Track {

this.trash.use(
await createVirtualTable(
this.engine,
this.trace.engine,
`raw_freq_idle_${this.trackUuid}`,
`span_join(raw_freq_${this.trackUuid}, raw_idle_${this.trackUuid})`,
),
Expand All @@ -122,7 +119,7 @@ export class CpuFreqTrack implements Track {

this.trash.use(
await createVirtualTable(
this.engine,
this.trace.engine,
`cpu_freq_${this.trackUuid}`,
`
__intrinsic_counter_mipmap((
Expand All @@ -135,7 +132,7 @@ export class CpuFreqTrack implements Track {

this.trash.use(
await createVirtualTable(
this.engine,
this.trace.engine,
`cpu_idle_${this.trackUuid}`,
`
__intrinsic_counter_mipmap((
Expand Down Expand Up @@ -167,7 +164,7 @@ export class CpuFreqTrack implements Track {
// function to make sense.
assertTrue(BIMath.popcount(resolution) === 1, `${resolution} not pow of 2`);

const freqResult = await this.engine.query(`
const freqResult = await this.trace.engine.query(`
SELECT
min_value as minFreq,
max_value as maxFreq,
Expand All @@ -179,7 +176,7 @@ export class CpuFreqTrack implements Track {
${resolution}
);
`);
const idleResult = await this.engine.query(`
const idleResult = await this.trace.engine.query(`
SELECT last_value as lastIdle
FROM cpu_idle_${this.trackUuid}(
${start},
Expand Down Expand Up @@ -257,7 +254,7 @@ export class CpuFreqTrack implements Track {

const color = colorForCpu(this.config.cpu);
let saturation = 45;
if (globals.state.hoveredUtid !== -1) {
if (this.trace.timeline.hoveredUtid !== undefined) {
saturation = 0;
}

Expand Down
2 changes: 1 addition & 1 deletion ui/src/core_plugins/cpu_freq/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class CpuFreq implements PerfettoPlugin {
kind: CPU_FREQ_TRACK_KIND,
cpu,
},
track: new CpuFreqTrack(config, ctx.engine),
track: new CpuFreqTrack(config, ctx),
});
const trackNode = new TrackNode({uri, title, sortOrder: -40});
ctx.workspace.addChildInOrder(trackNode);
Expand Down
21 changes: 10 additions & 11 deletions ui/src/core_plugins/cpu_slices/cpu_slice_track.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {BigintMath as BIMath} from '../../base/bigint_math';
import {search, searchEq, searchSegment} from '../../base/binary_search';
import {assertExists, assertTrue} from '../../base/logging';
import {Duration, duration, Time, time} from '../../base/time';
import {Actions} from '../../common/actions';
import {
drawDoubleHeadedArrow,
drawIncompleteSlice,
Expand All @@ -28,7 +27,6 @@ import {colorForThread} from '../../core/colorizer';
import {TrackData} from '../../common/track_data';
import {TimelineFetcher} from '../../common/track_helper';
import {checkerboardExcept} from '../../frontend/checkerboard';
import {globals} from '../../frontend/globals';
import {Point2D} from '../../base/geom';
import {Track} from '../../public/track';
import {LONG, NUM} from '../../trace_processor/query_result';
Expand Down Expand Up @@ -242,9 +240,9 @@ export class CpuSliceTrack implements Track {
// eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
const pid = threadInfo && threadInfo.pid ? threadInfo.pid : -1;

const isHovering = globals.state.hoveredUtid !== -1;
const isThreadHovered = globals.state.hoveredUtid === utid;
const isProcessHovered = globals.state.hoveredPid === pid;
const isHovering = this.trace.timeline.hoveredUtid !== undefined;
const isThreadHovered = this.trace.timeline.hoveredUtid === utid;
const isProcessHovered = this.trace.timeline.hoveredPid === pid;
const colorScheme = colorForThread(threadInfo);
let color: Color;
let textColor: Color;
Expand Down Expand Up @@ -314,7 +312,7 @@ export class CpuSliceTrack implements Track {
ctx.fillText(subTitle, rectXCenter, MARGIN_TOP + RECT_HEIGHT / 2 + 9);
}

const selection = globals.selectionManager.selection;
const selection = this.trace.selection.selection;
if (selection.kind === 'track_event') {
if (selection.trackUri === this.uri) {
const [startIndex, endIndex] = searchEq(data.ids, selection.eventId);
Expand Down Expand Up @@ -408,7 +406,8 @@ export class CpuSliceTrack implements Track {
if (data === undefined) return;
if (y < MARGIN_TOP || y > MARGIN_TOP + RECT_HEIGHT) {
this.utidHoveredInThisTrack = -1;
globals.dispatch(Actions.setHoveredUtidAndPid({utid: -1, pid: -1}));
this.trace.timeline.hoveredUtid = undefined;
this.trace.timeline.hoveredPid = undefined;
return;
}
const t = timescale.pxToHpTime(x);
Expand All @@ -427,14 +426,14 @@ export class CpuSliceTrack implements Track {
const threadInfo = this.trace.threads.get(hoveredUtid);
// eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
const hoveredPid = threadInfo ? (threadInfo.pid ? threadInfo.pid : -1) : -1;
globals.dispatch(
Actions.setHoveredUtidAndPid({utid: hoveredUtid, pid: hoveredPid}),
);
this.trace.timeline.hoveredUtid = hoveredUtid;
this.trace.timeline.hoveredPid = hoveredPid;
}

onMouseOut() {
this.utidHoveredInThisTrack = -1;
globals.dispatch(Actions.setHoveredUtidAndPid({utid: -1, pid: -1}));
this.trace.timeline.hoveredUtid = undefined;
this.trace.timeline.hoveredPid = undefined;
this.mousePos = undefined;
}

Expand Down
20 changes: 11 additions & 9 deletions ui/src/core_plugins/process_summary/process_scheduling_track.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,12 @@ import {BigintMath as BIMath} from '../../base/bigint_math';
import {searchEq, searchRange} from '../../base/binary_search';
import {assertExists, assertTrue} from '../../base/logging';
import {duration, time, Time} from '../../base/time';
import {Actions} from '../../common/actions';
import {drawTrackHoverTooltip} from '../../base/canvas_utils';
import {Color} from '../../public/color';
import {colorForThread} from '../../core/colorizer';
import {TrackData} from '../../common/track_data';
import {TimelineFetcher} from '../../common/track_helper';
import {checkerboardExcept} from '../../frontend/checkerboard';
import {globals} from '../../frontend/globals';
import {Track} from '../../public/track';
import {LONG, NUM, QueryResult} from '../../trace_processor/query_result';
import {uuidv4Sql} from '../../base/uuid';
Expand Down Expand Up @@ -231,9 +229,9 @@ export class ProcessSchedulingTrack implements Track {
// eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
const pid = (threadInfo ? threadInfo.pid : -1) || -1;

const isHovering = globals.state.hoveredUtid !== -1;
const isThreadHovered = globals.state.hoveredUtid === utid;
const isProcessHovered = globals.state.hoveredPid === pid;
const isHovering = this.trace.timeline.hoveredUtid !== undefined;
const isThreadHovered = this.trace.timeline.hoveredUtid === utid;
const isProcessHovered = this.trace.timeline.hoveredPid === pid;
const colorScheme = colorForThread(threadInfo);
let color: Color;
if (isHovering && !isThreadHovered) {
Expand Down Expand Up @@ -269,7 +267,8 @@ export class ProcessSchedulingTrack implements Track {
if (data === undefined) return;
if (y < MARGIN_TOP || y > MARGIN_TOP + RECT_HEIGHT) {
this.utidHoveredInThisTrack = -1;
globals.dispatch(Actions.setHoveredUtidAndPid({utid: -1, pid: -1}));
this.trace.timeline.hoveredUtid = undefined;
this.trace.timeline.hoveredPid = undefined;
return;
}

Expand All @@ -280,7 +279,8 @@ export class ProcessSchedulingTrack implements Track {
const [i, j] = searchRange(data.starts, t, searchEq(data.cpus, cpu));
if (i === j || i >= data.starts.length || t > data.ends[i]) {
this.utidHoveredInThisTrack = -1;
globals.dispatch(Actions.setHoveredUtidAndPid({utid: -1, pid: -1}));
this.trace.timeline.hoveredUtid = undefined;
this.trace.timeline.hoveredPid = undefined;
return;
}

Expand All @@ -289,12 +289,14 @@ export class ProcessSchedulingTrack implements Track {
const threadInfo = this.trace.threads.get(utid);
// eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
const pid = threadInfo ? (threadInfo.pid ? threadInfo.pid : -1) : -1;
globals.dispatch(Actions.setHoveredUtidAndPid({utid, pid}));
this.trace.timeline.hoveredUtid = utid;
this.trace.timeline.hoveredPid = pid;
}

onMouseOut() {
this.utidHoveredInThisTrack = -1;
globals.dispatch(Actions.setHoveredUtidAndPid({utid: -1, pid: -1}));
this.trace.timeline.hoveredUtid = undefined;
this.trace.timeline.hoveredPid = undefined;
this.mousePos = undefined;
}
}
Loading

0 comments on commit 7dfb6b5

Please sign in to comment.