Skip to content

Commit c57f6be

Browse files
altiminaltimin
committed
ui: refactor display of args
Leverage the new print_args() function to avoid reconstructing the args tree in JS.
1 parent 96c7193 commit c57f6be

File tree

9 files changed

+128
-173
lines changed

9 files changed

+128
-173
lines changed

‎ui/src/components/details/args.ts‎

Lines changed: 77 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -16,69 +16,98 @@ import m from 'mithril';
1616
import {isString} from '../../base/object_utils';
1717
import {Icons} from '../../base/semantic_icons';
1818
import {exists} from '../../base/utils';
19-
import {ArgNode, convertArgsToTree, Key} from './slice_args_parser';
2019
import {Anchor} from '../../widgets/anchor';
2120
import {MenuItem, PopupMenu} from '../../widgets/menu';
2221
import {TreeNode} from '../../widgets/tree';
23-
import {Arg} from '../sql_utils/args';
22+
import {Args, ArgsDict, ArgValue} from '../sql_utils/args';
2423
import {Trace} from '../../public/trace';
2524

2625
// Renders slice arguments (key/value pairs) as a subtree.
2726
export function renderArguments(
2827
trace: Trace,
29-
args: ReadonlyArray<Arg>,
30-
extraMenuItems?: (arg: Arg) => m.Children,
28+
args: ArgsDict,
29+
extraMenuItems?: (key: string, arg: ArgValue) => m.Children,
3130
): m.Children {
32-
if (args.length > 0) {
33-
const tree = convertArgsToTree(args);
34-
return renderArgTreeNodes(trace, tree, extraMenuItems);
35-
} else {
36-
return undefined;
31+
if (hasArgs(args)) {
32+
return Object.entries(args).map(([key, value]) =>
33+
renderArgsTree(trace, key, key, value, extraMenuItems),
34+
);
3735
}
36+
return undefined;
3837
}
3938

40-
export function hasArgs(args?: Arg[]): args is Arg[] {
41-
return exists(args) && args.length > 0;
39+
export function hasArgs(args?: ArgsDict): args is ArgsDict {
40+
return exists(args) && Object.keys(args).length > 0;
4241
}
4342

44-
function renderArgTreeNodes(
43+
function renderArgsTree(
4544
trace: Trace,
46-
args: ArgNode<Arg>[],
47-
extraMenuItems?: (arg: Arg) => m.Children,
45+
key: string,
46+
fullKey: string,
47+
args: Args,
48+
extraMenuItems?: (path: string, arg: ArgValue) => m.Children,
4849
): m.Children {
49-
return args.map((arg) => {
50-
const {key, value, children} = arg;
51-
if (children && children.length === 1) {
52-
// If we only have one child, collapse into self and combine keys
53-
const child = children[0];
54-
const compositeArg = {
55-
...child,
56-
key: stringifyKey(key, child.key),
57-
};
58-
return renderArgTreeNodes(trace, [compositeArg], extraMenuItems);
59-
} else {
60-
return m(
61-
TreeNode,
62-
{
63-
left: renderArgKey(stringifyKey(key), value, extraMenuItems),
64-
right: exists(value) && renderArgValue(value),
65-
summary: children && renderSummary(children),
66-
},
67-
children && renderArgTreeNodes(trace, children, extraMenuItems),
50+
if (args instanceof Array) {
51+
return m(
52+
TreeNode,
53+
{
54+
left: key,
55+
summary: renderArraySummary(args),
56+
},
57+
args.map((value, index) =>
58+
renderArgsTree(
59+
trace,
60+
`[${index}]`,
61+
`${fullKey}[${index}]`,
62+
value,
63+
extraMenuItems,
64+
),
65+
),
66+
);
67+
}
68+
if (args !== null && typeof args === 'object') {
69+
if (Object.keys(args).length === 1) {
70+
const [[childName, value]] = Object.entries(args);
71+
return renderArgsTree(
72+
trace,
73+
`${key}.${childName}`,
74+
`${fullKey}.${childName}`,
75+
value,
76+
extraMenuItems,
6877
);
6978
}
79+
return m(
80+
TreeNode,
81+
{
82+
left: key,
83+
summary: renderDictSummary(args),
84+
},
85+
Object.entries(args).map(([childName, child]) =>
86+
renderArgsTree(
87+
trace,
88+
childName,
89+
`${fullKey}.${childName}`,
90+
child,
91+
extraMenuItems,
92+
),
93+
),
94+
);
95+
}
96+
return m(TreeNode, {
97+
left: renderArgKey(key, fullKey, args, extraMenuItems),
98+
right: renderArgValue(args),
7099
});
71100
}
72101

73102
function renderArgKey(
74103
key: string,
75-
value: Arg | undefined,
76-
extraMenuItems?: (arg: Arg) => m.Children,
104+
fullKey: string,
105+
value: ArgValue,
106+
extraMenuItems?: (path: string, arg: ArgValue) => m.Children,
77107
): m.Children {
78108
if (value === undefined) {
79109
return key;
80110
} else {
81-
const {key: fullKey} = value;
82111
return m(
83112
PopupMenu,
84113
{trigger: m(Anchor, {icon: Icons.ContextMenu}, key)},
@@ -87,44 +116,33 @@ function renderArgKey(
87116
icon: 'content_copy',
88117
onclick: () => navigator.clipboard.writeText(fullKey),
89118
}),
90-
extraMenuItems?.(value),
119+
extraMenuItems?.(fullKey, value),
91120
);
92121
}
93122
}
94123

95-
function renderArgValue({displayValue}: Arg): m.Children {
96-
if (isWebLink(displayValue)) {
97-
return renderWebLink(displayValue);
124+
function renderArgValue(value: ArgValue): m.Children {
125+
if (isWebLink(value)) {
126+
return renderWebLink(value);
98127
} else {
99-
return `${displayValue}`;
128+
return `${value}`;
100129
}
101130
}
102131

103-
function renderSummary(children: ArgNode<Arg>[]): m.Children {
104-
const summary = children
105-
.slice(0, 2)
106-
.map(({key}) => key)
107-
.join(', ');
108-
const remaining = children.length - 2;
132+
function renderArraySummary(children: Args[]): m.Children {
133+
return `[ ... (${children.length} items ]`;
134+
}
135+
136+
function renderDictSummary(children: ArgsDict): m.Children {
137+
const summary = Object.keys(children).slice(0, 2).join(', ');
138+
const remaining = Object.keys(children).length - 2;
109139
if (remaining > 0) {
110140
return `{${summary}, ... (${remaining} more items)}`;
111141
} else {
112142
return `{${summary}}`;
113143
}
114144
}
115145

116-
function stringifyKey(...key: Key[]): string {
117-
return key
118-
.map((element, index) => {
119-
if (typeof element === 'number') {
120-
return `[${element}]`;
121-
} else {
122-
return (index === 0 ? '' : '.') + element;
123-
}
124-
})
125-
.join('');
126-
}
127-
128146
function isWebLink(value: unknown): value is string {
129147
return (
130148
isString(value) &&

‎ui/src/components/details/slice_args.ts‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
import m from 'mithril';
1616
import {MenuItem} from '../../widgets/menu';
17-
import {Arg} from '../sql_utils/args';
17+
import {ArgsDict} from '../sql_utils/args';
1818
import {Trace} from '../../public/trace';
1919
import {renderArguments} from './args';
2020
import {extensions} from '../extensions';
@@ -23,11 +23,9 @@ import {getSqlTableDescription} from '../widgets/sql/table/sql_table_registry';
2323
import {sqliteString} from '../../base/string_utils';
2424

2525
// Renders slice arguments (key/value pairs) as a subtree.
26-
export function renderSliceArguments(
27-
trace: Trace,
28-
args: ReadonlyArray<Arg>,
29-
): m.Children {
30-
return renderArguments(trace, args, (arg) => {
26+
export function renderSliceArguments(trace: Trace, args: ArgsDict): m.Children {
27+
return renderArguments(trace, args, (key, value) => {
28+
const displayValue = value === null ? 'NULL' : String(value);
3129
return [
3230
m(MenuItem, {
3331
label: 'Find slices with same arg value',
@@ -37,15 +35,15 @@ export function renderSliceArguments(
3735
table: assertExists(getSqlTableDescription(trace, 'slice')),
3836
filters: [
3937
{
40-
op: (cols) => `${cols[0]} = ${sqliteString(arg.displayValue)}`,
38+
op: (cols) => `${cols[0]} = ${sqliteString(displayValue)}`,
4139
columns: [
4240
{
4341
column: 'display_value',
4442
source: {
4543
table: 'args',
4644
joinOn: {
4745
arg_set_id: 'arg_set_id',
48-
key: sqliteString(arg.flatKey),
46+
key: sqliteString(key),
4947
},
5048
},
5149
},
@@ -59,7 +57,7 @@ export function renderSliceArguments(
5957
label: 'Visualize argument values',
6058
icon: 'query_stats',
6159
onclick: () => {
62-
extensions.addVisualizedArgTracks(trace, arg.flatKey);
60+
extensions.addVisualizedArgTracks(trace, key);
6361
},
6462
}),
6563
];

‎ui/src/components/sql_utils/args.ts‎

Lines changed: 14 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -13,58 +13,28 @@
1313
// limitations under the License.
1414

1515
import {Engine} from '../../trace_processor/engine';
16-
import {
17-
LONG_NULL,
18-
NUM,
19-
NUM_NULL,
20-
STR,
21-
STR_NULL,
22-
} from '../../trace_processor/query_result';
23-
import {ArgSetId, ArgsId, asArgId} from './core_types';
16+
import {STR_NULL} from '../../trace_processor/query_result';
17+
import {ArgSetId} from './core_types';
2418

25-
export interface Arg {
26-
id: ArgsId;
27-
flatKey: string;
28-
key: string;
29-
displayValue: string;
30-
}
19+
export type ArgValue = string | number | boolean | null;
20+
export type Args = ArgValue | Args[] | ArgsDict;
21+
export type ArgsDict = {[key: string]: Args};
3122

3223
export async function getArgs(
3324
engine: Engine,
3425
argSetId: ArgSetId,
35-
): Promise<Arg[]> {
26+
): Promise<ArgsDict> {
3627
const query = await engine.query(`
37-
SELECT
38-
id,
39-
flat_key as flatKey,
40-
key,
41-
int_value as intValue,
42-
string_value as stringValue,
43-
real_value as realValue,
44-
value_type as valueType,
45-
display_value as displayValue
46-
FROM args
47-
WHERE arg_set_id = ${argSetId}
48-
ORDER BY id`);
28+
SELECT print_args(${argSetId}) as args_json
29+
`);
4930
const it = query.iter({
50-
id: NUM,
51-
flatKey: STR,
52-
key: STR,
53-
intValue: LONG_NULL,
54-
stringValue: STR_NULL,
55-
realValue: NUM_NULL,
56-
valueType: STR,
57-
displayValue: STR_NULL,
31+
args_json: STR_NULL,
5832
});
5933

60-
const result: Arg[] = [];
61-
for (; it.valid(); it.next()) {
62-
result.push({
63-
id: asArgId(it.id),
64-
flatKey: it.flatKey,
65-
key: it.key,
66-
displayValue: it.displayValue ?? 'NULL',
67-
});
34+
if (!it.valid() || it.args_json === null) {
35+
return {};
6836
}
69-
return result;
37+
38+
const argsDict = JSON.parse(it.args_json);
39+
return argsDict;
7040
}

‎ui/src/components/sql_utils/slice.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import {
3636
Upid,
3737
Utid,
3838
} from './core_types';
39-
import {Arg, getArgs} from './args';
39+
import {ArgsDict, getArgs} from './args';
4040
import {getThreadInfo, ThreadInfo} from './thread';
4141
import {getProcessInfo, ProcessInfo} from './process';
4242

@@ -55,7 +55,7 @@ export interface SliceDetails {
5555
threadTs?: time;
5656
threadDur?: duration;
5757
category?: string;
58-
args?: Arg[];
58+
args?: ArgsDict;
5959
}
6060

6161
async function getUtidAndUpid(

‎ui/src/components/tracks/debug_slice_track_details_panel.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ import {TrackEventDetailsPanel} from '../../public/details_panel';
4141
import {Trace} from '../../public/trace';
4242
import {SqlRef} from '../../widgets/sql_ref';
4343
import {renderSliceArguments} from '../details/slice_args';
44-
import {Arg, getArgs} from '../sql_utils/args';
44+
import {ArgsDict, getArgs} from '../sql_utils/args';
4545

4646
export const RAW_PREFIX = 'raw_';
4747

@@ -81,7 +81,7 @@ export class DebugSliceTrackDetailsPanel implements TrackEventDetailsPanel {
8181

8282
// These are the actual loaded args from the args table assuming an arg_set_id
8383
// is supplied.
84-
private args?: Arg[];
84+
private args?: ArgsDict;
8585

8686
// We will try to interpret the arguments as references into well-known
8787
// tables. These values will be set if the relevant columns exist and

‎ui/src/components/widgets/sql/details/details.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {raf} from '../../../../core/raf_scheduler';
2020
import {Engine} from '../../../../trace_processor/engine';
2121
import {Row, SqlValue} from '../../../../trace_processor/query_result';
2222
import {sqlValueToReadableString} from '../../../../trace_processor/sql_utils';
23-
import {Arg, getArgs} from '../../../sql_utils/args';
23+
import {ArgsDict, getArgs} from '../../../sql_utils/args';
2424
import {asArgSetId} from '../../../sql_utils/core_types';
2525
import {Anchor} from '../../../../widgets/anchor';
2626
import {renderError} from '../../../../widgets/error';
@@ -432,7 +432,7 @@ interface Data {
432432
// Source statements for the arg sets.
433433
argSetExpressions: string[];
434434
// Fetched arg sets.
435-
argSets: (Arg[] | Err)[];
435+
argSets: (ArgsDict | Err)[];
436436

437437
// Source statements for the SQL references.
438438
sqlIdRefs: {tableName: string; idExpression: string}[];
@@ -508,7 +508,7 @@ class DataController {
508508
for (const argSetIndex of this.argSets) {
509509
const argSetId = data.values[argSetIndex];
510510
if (argSetId === null) {
511-
data.argSets.push([]);
511+
data.argSets.push({});
512512
} else if (typeof argSetId !== 'number' && typeof argSetId !== 'bigint') {
513513
data.argSets.push(
514514
new Err(

0 commit comments

Comments
 (0)