Skip to content

Commit a6a65d0

Browse files
gitstergitster
committed
Merge branch 'jh/trace2-pretty-output' into pu
Output from trace2 subsystem is formatted more prettily now. * jh/trace2-pretty-output: trace2: cleanup whitespace in perf format trace2: cleanup whitespace in normal format quote: add sq_quote_argv_pretty_ltrim trace2: trim trailing whitespace in normal format error message trace2: remove dead code in maybe_add_string_va() trace2: trim whitespace in region messages in perf target format trace2: cleanup column alignment in perf target format
2 parents 129e261 + 1964382 commit a6a65d0

File tree

6 files changed

+90
-53
lines changed

6 files changed

+90
-53
lines changed

‎quote.c‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,17 @@ void sq_quote_argv_pretty(struct strbuf *dst, const char **argv)
9494
}
9595
}
9696

97+
void sq_quote_argv_pretty_ltrim(struct strbuf *dst, const char **argv)
98+
{
99+
int i;
100+
101+
for (i = 0; argv[i]; i++) {
102+
if (i > 0)
103+
strbuf_addch(dst, ' ');
104+
sq_quote_buf_pretty(dst, argv[i]);
105+
}
106+
}
107+
97108
static char *sq_dequote_step(char *arg, char **next)
98109
{
99110
char *dst = arg;

‎quote.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ void sq_quotef(struct strbuf *, const char *fmt, ...);
4040
*/
4141
void sq_quote_buf_pretty(struct strbuf *, const char *src);
4242
void sq_quote_argv_pretty(struct strbuf *, const char **argv);
43+
void sq_quote_argv_pretty_ltrim(struct strbuf *, const char **argv);
4344

4445
/* This unwraps what sq_quote() produces in place, but returns
4546
* NULL if the input does not look like what sq_quote would have

‎t/t0211-trace2-perf.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,11 +130,11 @@ test_expect_success 'perf stream, child processes' '
130130
d0|main|version|||||$V
131131
d0|main|start||_T_ABS_|||_EXE_ trace2 004child test-tool trace2 004child test-tool trace2 001return 0
132132
d0|main|cmd_name|||||trace2 (trace2)
133-
d0|main|child_start||_T_ABS_|||[ch0] class:? argv: test-tool trace2 004child test-tool trace2 001return 0
133+
d0|main|child_start||_T_ABS_|||[ch0] class:? argv:[test-tool trace2 004child test-tool trace2 001return 0]
134134
d1|main|version|||||$V
135135
d1|main|start||_T_ABS_|||_EXE_ trace2 004child test-tool trace2 001return 0
136136
d1|main|cmd_name|||||trace2 (trace2/trace2)
137-
d1|main|child_start||_T_ABS_|||[ch0] class:? argv: test-tool trace2 001return 0
137+
d1|main|child_start||_T_ABS_|||[ch0] class:? argv:[test-tool trace2 001return 0]
138138
d2|main|version|||||$V
139139
d2|main|start||_T_ABS_|||_EXE_ trace2 001return 0
140140
d2|main|cmd_name|||||trace2 (trace2/trace2/trace2)

‎trace2/tr2_tgt_event.c‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -205,11 +205,6 @@ static void maybe_add_string_va(struct json_writer *jw, const char *field_name,
205205
strbuf_release(&buf);
206206
return;
207207
}
208-
209-
if (fmt && *fmt) {
210-
jw_object_string(jw, field_name, fmt);
211-
return;
212-
}
213208
}
214209

215210
static void fn_error_va_fl(const char *file, int line, const char *fmt,

‎trace2/tr2_tgt_normal.c‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ static void fn_start_fl(const char *file, int line,
8787
struct strbuf buf_payload = STRBUF_INIT;
8888

8989
strbuf_addstr(&buf_payload, "start ");
90-
sq_quote_argv_pretty(&buf_payload, argv);
90+
sq_quote_argv_pretty_ltrim(&buf_payload, argv);
9191
normal_io_write_fl(file, line, &buf_payload);
9292
strbuf_release(&buf_payload);
9393
}
@@ -135,20 +135,18 @@ static void maybe_append_string_va(struct strbuf *buf, const char *fmt,
135135
va_end(copy_ap);
136136
return;
137137
}
138-
139-
if (fmt && *fmt) {
140-
strbuf_addstr(buf, fmt);
141-
return;
142-
}
143138
}
144139

145140
static void fn_error_va_fl(const char *file, int line, const char *fmt,
146141
va_list ap)
147142
{
148143
struct strbuf buf_payload = STRBUF_INIT;
149144

150-
strbuf_addstr(&buf_payload, "error ");
151-
maybe_append_string_va(&buf_payload, fmt, ap);
145+
strbuf_addstr(&buf_payload, "error");
146+
if (fmt && *fmt) {
147+
strbuf_addch(&buf_payload, ' ');
148+
maybe_append_string_va(&buf_payload, fmt, ap);
149+
}
152150
normal_io_write_fl(file, line, &buf_payload);
153151
strbuf_release(&buf_payload);
154152
}
@@ -188,8 +186,8 @@ static void fn_alias_fl(const char *file, int line, const char *alias,
188186
{
189187
struct strbuf buf_payload = STRBUF_INIT;
190188

191-
strbuf_addf(&buf_payload, "alias %s ->", alias);
192-
sq_quote_argv_pretty(&buf_payload, argv);
189+
strbuf_addf(&buf_payload, "alias %s -> ", alias);
190+
sq_quote_argv_pretty_ltrim(&buf_payload, argv);
193191
normal_io_write_fl(file, line, &buf_payload);
194192
strbuf_release(&buf_payload);
195193
}
@@ -200,22 +198,23 @@ static void fn_child_start_fl(const char *file, int line,
200198
{
201199
struct strbuf buf_payload = STRBUF_INIT;
202200

203-
strbuf_addf(&buf_payload, "child_start[%d] ", cmd->trace2_child_id);
201+
strbuf_addf(&buf_payload, "child_start[%d]", cmd->trace2_child_id);
204202

205203
if (cmd->dir) {
206204
strbuf_addstr(&buf_payload, " cd");
207205
sq_quote_buf_pretty(&buf_payload, cmd->dir);
208-
strbuf_addstr(&buf_payload, "; ");
206+
strbuf_addstr(&buf_payload, ";");
209207
}
210208

211209
/*
212210
* TODO if (cmd->env) { Consider dumping changes to environment. }
213211
* See trace_add_env() in run-command.c as used by original trace.c
214212
*/
215213

214+
strbuf_addch(&buf_payload, ' ');
216215
if (cmd->git_cmd)
217-
strbuf_addstr(&buf_payload, "git");
218-
sq_quote_argv_pretty(&buf_payload, cmd->argv);
216+
strbuf_addstr(&buf_payload, "git ");
217+
sq_quote_argv_pretty_ltrim(&buf_payload, cmd->argv);
219218

220219
normal_io_write_fl(file, line, &buf_payload);
221220
strbuf_release(&buf_payload);
@@ -240,9 +239,11 @@ static void fn_exec_fl(const char *file, int line, uint64_t us_elapsed_absolute,
240239
struct strbuf buf_payload = STRBUF_INIT;
241240

242241
strbuf_addf(&buf_payload, "exec[%d] ", exec_id);
243-
if (exe)
242+
if (exe) {
244243
strbuf_addstr(&buf_payload, exe);
245-
sq_quote_argv_pretty(&buf_payload, argv);
244+
strbuf_addch(&buf_payload, ' ');
245+
}
246+
sq_quote_argv_pretty_ltrim(&buf_payload, argv);
246247
normal_io_write_fl(file, line, &buf_payload);
247248
strbuf_release(&buf_payload);
248249
}

‎trace2/tr2_tgt_perf.c‎

Lines changed: 59 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ static struct tr2_dst tr2dst_perf = { TR2_SYSENV_PERF, 0, 0, 0 };
2121
*/
2222
static int tr2env_perf_be_brief;
2323

24-
#define TR2FMT_PERF_FL_WIDTH (50)
24+
#define TR2FMT_PERF_FL_WIDTH (28)
2525
#define TR2FMT_PERF_MAX_EVENT_NAME (12)
26-
#define TR2FMT_PERF_REPO_WIDTH (4)
27-
#define TR2FMT_PERF_CATEGORY_WIDTH (10)
26+
#define TR2FMT_PERF_REPO_WIDTH (3)
27+
#define TR2FMT_PERF_CATEGORY_WIDTH (12)
2828

2929
#define TR2_DOTS_BUFFER_SIZE (100)
3030
#define TR2_INDENT (2)
@@ -79,17 +79,36 @@ static void perf_fmt_prepare(const char *event_name,
7979

8080
if (!tr2env_perf_be_brief) {
8181
struct tr2_tbuf tb_now;
82+
size_t fl_end_col;
8283

8384
tr2_tbuf_local_time(&tb_now);
8485
strbuf_addstr(buf, tb_now.buf);
8586
strbuf_addch(buf, ' ');
8687

87-
if (file && *file)
88-
strbuf_addf(buf, "%s:%d ", file, line);
89-
while (buf->len < TR2FMT_PERF_FL_WIDTH)
88+
fl_end_col = buf->len + TR2FMT_PERF_FL_WIDTH;
89+
90+
if (file && *file) {
91+
struct strbuf buf_fl = STRBUF_INIT;
92+
93+
strbuf_addf(&buf_fl, "%s:%d", file, line);
94+
95+
if (buf_fl.len <= TR2FMT_PERF_FL_WIDTH)
96+
strbuf_addbuf(buf, &buf_fl);
97+
else {
98+
size_t avail = TR2FMT_PERF_FL_WIDTH - 3;
99+
strbuf_addstr(buf, "...");
100+
strbuf_add(buf,
101+
&buf_fl.buf[buf_fl.len - avail],
102+
avail);
103+
}
104+
105+
strbuf_release(&buf_fl);
106+
}
107+
108+
while (buf->len < fl_end_col)
90109
strbuf_addch(buf, ' ');
91110

92-
strbuf_addstr(buf, "| ");
111+
strbuf_addstr(buf, " | ");
93112
}
94113

95114
strbuf_addf(buf, "d%d | ", tr2_sid_depth());
@@ -102,7 +121,7 @@ static void perf_fmt_prepare(const char *event_name,
102121
strbuf_addf(buf, "r%d ", repo->trace2_repo_id);
103122
while (buf->len < len)
104123
strbuf_addch(buf, ' ');
105-
strbuf_addstr(buf, "| ");
124+
strbuf_addstr(buf, " | ");
106125

107126
if (p_us_elapsed_absolute)
108127
strbuf_addf(buf, "%9.6f | ",
@@ -116,8 +135,8 @@ static void perf_fmt_prepare(const char *event_name,
116135
else
117136
strbuf_addf(buf, "%9s | ", " ");
118137

119-
strbuf_addf(buf, "%-*s | ", TR2FMT_PERF_CATEGORY_WIDTH,
120-
(category ? category : ""));
138+
strbuf_addf(buf, "%-*.*s | ", TR2FMT_PERF_CATEGORY_WIDTH,
139+
TR2FMT_PERF_CATEGORY_WIDTH, (category ? category : ""));
121140

122141
if (ctx->nr_open_regions > 0) {
123142
int len_indent = TR2_INDENT_LENGTH(ctx);
@@ -165,7 +184,7 @@ static void fn_start_fl(const char *file, int line,
165184
const char *event_name = "start";
166185
struct strbuf buf_payload = STRBUF_INIT;
167186

168-
sq_quote_argv_pretty(&buf_payload, argv);
187+
sq_quote_argv_pretty_ltrim(&buf_payload, argv);
169188

170189
perf_io_write_fl(file, line, event_name, NULL, &us_elapsed_absolute,
171190
NULL, NULL, &buf_payload);
@@ -220,11 +239,6 @@ static void maybe_append_string_va(struct strbuf *buf, const char *fmt,
220239
va_end(copy_ap);
221240
return;
222241
}
223-
224-
if (fmt && *fmt) {
225-
strbuf_addstr(buf, fmt);
226-
return;
227-
}
228242
}
229243

230244
static void fn_error_va_fl(const char *file, int line, const char *fmt,
@@ -285,8 +299,9 @@ static void fn_alias_fl(const char *file, int line, const char *alias,
285299
const char *event_name = "alias";
286300
struct strbuf buf_payload = STRBUF_INIT;
287301

288-
strbuf_addf(&buf_payload, "alias:%s argv:", alias);
289-
sq_quote_argv_pretty(&buf_payload, argv);
302+
strbuf_addf(&buf_payload, "alias:%s argv:[", alias);
303+
sq_quote_argv_pretty_ltrim(&buf_payload, argv);
304+
strbuf_addch(&buf_payload, ']');
290305

291306
perf_io_write_fl(file, line, event_name, NULL, NULL, NULL, NULL,
292307
&buf_payload);
@@ -315,10 +330,14 @@ static void fn_child_start_fl(const char *file, int line,
315330
sq_quote_buf_pretty(&buf_payload, cmd->dir);
316331
}
317332

318-
strbuf_addstr(&buf_payload, " argv:");
319-
if (cmd->git_cmd)
320-
strbuf_addstr(&buf_payload, " git");
321-
sq_quote_argv_pretty(&buf_payload, cmd->argv);
333+
strbuf_addstr(&buf_payload, " argv:[");
334+
if (cmd->git_cmd) {
335+
strbuf_addstr(&buf_payload, "git");
336+
if (cmd->argv[0])
337+
strbuf_addch(&buf_payload, ' ');
338+
}
339+
sq_quote_argv_pretty_ltrim(&buf_payload, cmd->argv);
340+
strbuf_addch(&buf_payload, ']');
322341

323342
perf_io_write_fl(file, line, event_name, NULL, &us_elapsed_absolute,
324343
NULL, NULL, &buf_payload);
@@ -369,10 +388,14 @@ static void fn_exec_fl(const char *file, int line, uint64_t us_elapsed_absolute,
369388
struct strbuf buf_payload = STRBUF_INIT;
370389

371390
strbuf_addf(&buf_payload, "id:%d ", exec_id);
372-
strbuf_addstr(&buf_payload, "argv:");
373-
if (exe)
374-
strbuf_addf(&buf_payload, " %s", exe);
375-
sq_quote_argv_pretty(&buf_payload, argv);
391+
strbuf_addstr(&buf_payload, "argv:[");
392+
if (exe) {
393+
strbuf_addstr(&buf_payload, exe);
394+
if (argv[0])
395+
strbuf_addch(&buf_payload, ' ');
396+
}
397+
sq_quote_argv_pretty_ltrim(&buf_payload, argv);
398+
strbuf_addch(&buf_payload, ']');
376399

377400
perf_io_write_fl(file, line, event_name, NULL, &us_elapsed_absolute,
378401
NULL, NULL, &buf_payload);
@@ -433,8 +456,11 @@ static void fn_region_enter_printf_va_fl(const char *file, int line,
433456
struct strbuf buf_payload = STRBUF_INIT;
434457

435458
if (label)
436-
strbuf_addf(&buf_payload, "label:%s ", label);
437-
maybe_append_string_va(&buf_payload, fmt, ap);
459+
strbuf_addf(&buf_payload, "label:%s", label);
460+
if (fmt && *fmt) {
461+
strbuf_addch(&buf_payload, ' ');
462+
maybe_append_string_va(&buf_payload, fmt, ap);
463+
}
438464

439465
perf_io_write_fl(file, line, event_name, repo, &us_elapsed_absolute,
440466
NULL, category, &buf_payload);
@@ -450,8 +476,11 @@ static void fn_region_leave_printf_va_fl(
450476
struct strbuf buf_payload = STRBUF_INIT;
451477

452478
if (label)
453-
strbuf_addf(&buf_payload, "label:%s ", label);
454-
maybe_append_string_va(&buf_payload, fmt, ap);
479+
strbuf_addf(&buf_payload, "label:%s", label);
480+
if (fmt && *fmt) {
481+
strbuf_addch(&buf_payload, ' ' );
482+
maybe_append_string_va(&buf_payload, fmt, ap);
483+
}
455484

456485
perf_io_write_fl(file, line, event_name, repo, &us_elapsed_absolute,
457486
&us_elapsed_region, category, &buf_payload);

0 commit comments

Comments
 (0)