From 12ebaf44752440556467981a26faf29b465fafec Mon Sep 17 00:00:00 2001 From: Hector Dearman Date: Wed, 18 Oct 2023 16:32:54 +0100 Subject: [PATCH] ui: Don't force tracks to always create views - Replace required abstract method initSqlTable with two new methods: getSqlExpression and the optional onInit. - getSqlExpression returns an SQL expression that we can use whereever we used to use tableName - onInit allows for more elaborate setup if needed. - Simplify the story for teardown. Rather than replying on users to override onDestory, onInit now returns a disposable which the base class calls from onDestory. - Now most users never need to care about a) tableName or b) teardown. Change-Id: Idc11a2a7d1543bd68acd5fead7e16dae91a2aabe --- ui/src/base/disposable.ts | 3 ++ ui/src/frontend/base_slice_track.ts | 50 +++++++++++++------ .../chrome_tasks_scroll_jank_track.ts | 13 ++--- .../chrome_scroll_jank/event_latency_track.ts | 7 +-- .../chrome_slices/generic_slice_track.ts | 6 +-- .../tracks/custom_sql_table_slices/index.ts | 30 +++++++---- ui/src/tracks/debug/slice_track.ts | 5 +- .../thread_state/thread_state_track_v2.ts | 5 +- ui/src/tracks/thread_state/thread_state_v2.ts | 6 +-- 9 files changed, 77 insertions(+), 48 deletions(-) diff --git a/ui/src/base/disposable.ts b/ui/src/base/disposable.ts index cc5e397e9b..15b0346548 100644 --- a/ui/src/base/disposable.ts +++ b/ui/src/base/disposable.ts @@ -51,6 +51,9 @@ export class DisposableCallback implements Disposable { } } +export class NullDisposable implements Disposable { + dispose() {} +} // A collection of Disposables. // Disposables can be added one by one, (e.g. during the lifecycle of a diff --git a/ui/src/frontend/base_slice_track.ts b/ui/src/frontend/base_slice_track.ts index f932269a14..efef610d4e 100644 --- a/ui/src/frontend/base_slice_track.ts +++ b/ui/src/frontend/base_slice_track.ts @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +import {Disposable, NullDisposable} from '../base/disposable'; import {assertExists} from '../base/logging'; import { duration, @@ -144,12 +145,14 @@ export const filterVisibleSlicesForTesting = filterVisibleSlices; // If you need temporally overlapping slices, look at AsyncSliceTrack, which // merges several tracks into one visual track. export const BASE_SLICE_ROW = { - id: NUM, // The slice ID, for selection / lookups. + id: NUM, // The slice ID, for selection / lookups. + ts: LONG, // Start time in nanoseconds. + dur: LONG, // Duration in nanoseconds. -1 = incomplete, 0 = instant. + depth: NUM, // Vertical depth. + + // These are computed by the base class: tsq: LONG, // Quantized |ts|. This class owns the quantization logic. tsqEnd: LONG, // Quantized |ts+dur|. The end bucket. - ts: LONG, // Start time in nanoseconds. - dur: LONG, // Duration in nanoseconds. -1 = incomplete, 0 = instant. - depth: NUM, // Vertical depth. }; export type BaseSliceRow = typeof BASE_SLICE_ROW; @@ -203,7 +206,6 @@ export abstract class BaseSliceTrack; - protected readonly tableName: string; private maxDurNs: duration = 0n; private sqlState: 'UNINITIALIZED'|'INITIALIZING'|'QUERY_PENDING'| @@ -227,11 +229,30 @@ export abstract class BaseSliceTrack; + + // onInit hook lets you do asynchronous set up e.g. creating a table + // etc. We guarantee that this will be resolved before doing any + // queries using the result of getSqlSource(). All persistent + // state in trace_processor should be cleaned up when dispose is + // called on the returned hook. In the common case of where + // the data for this track is d + async onInit(): Promise { + return new NullDisposable(); + } + + // This should be an SQL expression returning all the columns listed + // metioned by getRowSpec() exluding tsq and tsqEnd. + // For example you might return an SQL expression of the form: + // `select id, ts, dur, 0 as depth from foo where bar = 'baz'` + abstract getSqlSource(): string; + getRowSpec(): T['row'] { return BASE_SLICE_ROW; } @@ -256,10 +277,6 @@ export abstract class BaseSliceTrack string; diff --git a/ui/src/tracks/chrome_scroll_jank/event_latency_track.ts b/ui/src/tracks/chrome_scroll_jank/event_latency_track.ts index 539d782c64..6e6c618d13 100644 --- a/ui/src/tracks/chrome_scroll_jank/event_latency_track.ts +++ b/ui/src/tracks/chrome_scroll_jank/event_latency_track.ts @@ -67,11 +67,8 @@ export class EventLatencyTrack extends ScrollJankPluginState.getInstance().unregisterTrack(EventLatencyTrack.kind); } - async initSqlTable(tableName: string) { - const sql = - `CREATE VIEW ${tableName} AS SELECT * FROM ${this.config.baseTable}`; - - await this.engine.query(sql); + getSqlSource(): string { + return `SELECT * FROM ${this.config.baseTable}`; } getDetailsPanel(): CustomSqlDetailsPanelConfig { diff --git a/ui/src/tracks/chrome_slices/generic_slice_track.ts b/ui/src/tracks/chrome_slices/generic_slice_track.ts index 802236bf4d..d19f3dba36 100644 --- a/ui/src/tracks/chrome_slices/generic_slice_track.ts +++ b/ui/src/tracks/chrome_slices/generic_slice_track.ts @@ -36,10 +36,8 @@ export class GenericSliceTrack extends NamedSliceTrack { super(args); } - async initSqlTable(tableName: string): Promise { - const sql = `create view ${tableName} as - select ts, dur, id, depth, ifnull(name, '') as name + getSqlSource(): string { + return `select ts, dur, id, depth, ifnull(name, '') as name from slice where track_id = ${this.config.sqlTrackId}`; - await this.engine.query(sql); } } diff --git a/ui/src/tracks/custom_sql_table_slices/index.ts b/ui/src/tracks/custom_sql_table_slices/index.ts index 8ffe3881e6..f8bd9918cb 100644 --- a/ui/src/tracks/custom_sql_table_slices/index.ts +++ b/ui/src/tracks/custom_sql_table_slices/index.ts @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +import {Disposable, DisposableCallback} from '../../base/disposable'; import {Actions} from '../../common/actions'; import { generateSqlWithInternalLayout, @@ -46,8 +47,11 @@ export interface CustomSqlDetailsPanelConfig { export abstract class CustomSqlTableSliceTrack< T extends NamedSliceTrackTypes> extends NamedSliceTrack { + protected readonly tableName; + constructor(args: NewTrackArgs) { super(args); + this.tableName = `customsqltableslicetrack_{uuidv4()}`; } abstract getSqlDataSource(): CustomSqlTableDefConfig; @@ -55,22 +59,30 @@ export abstract class CustomSqlTableSliceTrack< // Override by subclasses. abstract getDetailsPanel(): CustomSqlDetailsPanelConfig; - async initSqlTable(tableName: string) { + + async onInit(): Promise { const config = this.getSqlDataSource(); let columns = ['*']; if (config.columns !== undefined) { columns = config.columns; } - const sql = `CREATE VIEW ${tableName} AS ` + generateSqlWithInternalLayout({ - columns: columns, - sourceTable: config.sqlTableName, - ts: 'ts', - dur: 'dur', - whereClause: config.whereClause, - }); - + const sql = + `CREATE VIEW ${this.tableName} AS ` + generateSqlWithInternalLayout({ + columns: columns, + sourceTable: config.sqlTableName, + ts: 'ts', + dur: 'dur', + whereClause: config.whereClause, + }); await this.engine.query(sql); + return DisposableCallback.from(() => { + this.engine.query(`DROP VIEW ${this.tableName}`); + }); + } + + getSqlSource(): string { + return `SELECT * FROM ${this.tableName}`; } isSelectionHandled(selection: Selection) { diff --git a/ui/src/tracks/debug/slice_track.ts b/ui/src/tracks/debug/slice_track.ts index 96403128d7..e05fdafbbe 100644 --- a/ui/src/tracks/debug/slice_track.ts +++ b/ui/src/tracks/debug/slice_track.ts @@ -14,6 +14,7 @@ import m from 'mithril'; +import {Disposable} from '../../base/disposable'; import {Actions, DEBUG_SLICE_TRACK_KIND} from '../../common/actions'; import {EngineProxy} from '../../common/engine'; import {SCROLLING_TRACK_GROUP} from '../../common/state'; @@ -77,8 +78,8 @@ export class DebugTrackV2 extends CustomSqlTableSliceTrack { }; } - async initSqlTable(tableName: string): Promise { - super.initSqlTable(tableName); + async onInit(): Promise { + return super.onInit(); } getTrackShellButtons(): m.Children { diff --git a/ui/src/tracks/thread_state/thread_state_track_v2.ts b/ui/src/tracks/thread_state/thread_state_track_v2.ts index 59739604e4..3fe17f2ae6 100644 --- a/ui/src/tracks/thread_state/thread_state_track_v2.ts +++ b/ui/src/tracks/thread_state/thread_state_track_v2.ts @@ -63,10 +63,9 @@ export class ThreadStateTrack extends BaseSliceTrack { return THREAD_STATE_ROW; } - async initSqlTable(tableName: string): Promise { + getSqlSource(): string { // Do not display states 'x' and 'S' (dead & sleeping). const sql = ` - create view ${tableName} as select id, ts, @@ -81,7 +80,7 @@ export class ThreadStateTrack extends BaseSliceTrack { state != 'x' and state != 'S' `; - await this.engine.query(sql); + return sql; } rowToSlice(row: ThreadStateTrackTypes['row']): diff --git a/ui/src/tracks/thread_state/thread_state_v2.ts b/ui/src/tracks/thread_state/thread_state_v2.ts index 5f17530399..1ec933bac4 100644 --- a/ui/src/tracks/thread_state/thread_state_v2.ts +++ b/ui/src/tracks/thread_state/thread_state_v2.ts @@ -65,10 +65,9 @@ export class ThreadStateTrack extends BaseSliceTrack { return THREAD_STATE_ROW; } - async initSqlTable(tableName: string): Promise { + getSqlSource(): string { // Do not display states 'x' and 'S' (dead & sleeping). - const sql = ` - create view ${tableName} as + return ` select id, ts, @@ -83,7 +82,6 @@ export class ThreadStateTrack extends BaseSliceTrack { state != 'x' and state != 'S' `; - await this.engine.query(sql); } rowToSlice(row: ThreadStateTrackTypes['row']):