Re: [PATCH] perf/script: remove extraneous newline in perf_sample__fprintf_regs()

2020-04-29 Thread Stephane Eranian
On Wed, Apr 29, 2020 at 7:09 PM Andi Kleen  wrote:
>
> > I was under the impression that perf script was generating one line
> > per sample. Otherwise, seems hard to parse.
>
> That's only true for simple cases. A lot of the extended output options
> have long generated multiple lines. And of course call stacks always did.
>
> > Could you give me the cmdline options of perf script that justify the 
> > newline.
>
> e.g.  perf script -F iregs,uregs
>
But then it should only use the \n when needed.
It is a bit like perf stat printing cgroup as "" when not using cgroup
mode add a bunch of white spaces/tab at the end of the line for
nothing.

I also suggest that we improve perf stat/script output with an output
format that is easier to parse, such as JSON with name: value pairs.
that would avoid all these \n and
flaky parsing regexp or script I have seen, even internally.

> -Andi


Re: [PATCH] perf/script: remove extraneous newline in perf_sample__fprintf_regs()

2020-04-29 Thread Andi Kleen
> I was under the impression that perf script was generating one line
> per sample. Otherwise, seems hard to parse.

That's only true for simple cases. A lot of the extended output options
have long generated multiple lines. And of course call stacks always did.

> Could you give me the cmdline options of perf script that justify the newline.

e.g.  perf script -F iregs,uregs

-Andi


Re: [PATCH] perf/script: remove extraneous newline in perf_sample__fprintf_regs()

2020-04-29 Thread Stephane Eranian
On Mon, Apr 27, 2020 at 7:47 PM Andi Kleen  wrote:
>
> On Sat, Apr 18, 2020 at 04:19:08PM -0700, Stephane Eranian wrote:
> > When printing iregs, there was a double newline printed because
> > perf_sample__fprintf_regs() was printing its own and then at the
> > end of all fields, perf script was adding one.
> > This was causing blank line in the output:
>
> I don't think the patch is quite correct because there could be
> other fields after it, and they need to be separated by a
> new line too.
>
> e.g. i suspect if someone prints iregs,uregs or iregs,brstack
> or something else that is printed in process_event after *regs
> you would get garbled output.
>
I was under the impression that perf script was generating one line
per sample. Otherwise, seems hard to parse.
Could you give me the cmdline options of perf script that justify the newline.
 Thanks.

> So you have to track if the newline is needed or not.
>
> -Andi


Re: [PATCH] perf/script: remove extraneous newline in perf_sample__fprintf_regs()

2020-04-28 Thread Arnaldo Carvalho de Melo
Em Mon, Apr 27, 2020 at 02:43:28PM -0700, Stephane Eranian escreveu:
> On Sat, Apr 18, 2020 at 4:19 PM Stephane Eranian  wrote:
> >
> > When printing iregs, there was a double newline printed because
> > perf_sample__fprintf_regs() was printing its own and then at the
> > end of all fields, perf script was adding one.
> > This was causing blank line in the output:
> >
> > Before:
> > $ perf script -Fip,iregs
> >401b8d ABI:2DX:0x100SI:0x4a8340DI:0x4a9340
> >
> >401b8d ABI:2DX:0x100SI:0x4a9340DI:0x4a8340
> >
> >401b8d ABI:2DX:0x100SI:0x4a8340DI:0x4a9340
> >
> >401b8d ABI:2DX:0x100SI:0x4a9340DI:0x4a8340
> >
> > After:
> > $ perf script -Fip,iregs
> >401b8d ABI:2DX:0x100SI:0x4a8340DI:0x4a9340
> >401b8d ABI:2DX:0x100SI:0x4a9340DI:0x4a8340
> >401b8d ABI:2DX:0x100SI:0x4a8340DI:0x4a9340
> >
> > Signed-off-by: Stephane Eranian 
> 
> 
> Ping?

I'll process this one today.

Thanks for the reminder,

- Arnaldo
 
> >
> > ---
> >  tools/perf/builtin-script.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index 1f57a7ecdf3d0..0c0b6e807d06e 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -604,8 +604,6 @@ static int perf_sample__fprintf_regs(struct regs_dump 
> > *regs, uint64_t mask,
> > printed += fprintf(fp, "%5s:0x%"PRIx64" ", 
> > perf_reg_name(r), val);
> > }
> >
> > -   fprintf(fp, "\n");
> > -
> > return printed;
> >  }
> >
> > --
> > 2.26.1.301.g55bc3eb7cb9-goog
> >