[PATCH] D78903: [Driver] Add option -fproc-stat-report

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added inline comments.


================
Comment at: clang/docs/UsersManual.rst:770
+   $ cat abc
+   clang-11,"/tmp/foo-123456.o",92000,84000,87536
+   ld,"a.out",900,8000,53568
----------------
sepavloff wrote:
> aganea wrote:
> > Please add a header to the output .CSV, specifying the units of measure where relevant.
> CSV was chosen because such file is a simple union of records. It is convenient if multiple processes write to it. A process locks the file, writes to it and unlock it.
> 
> If header is required, thing get more complicated. A process locks file, then moves to the end and get current position. If it is the beginning of the file, this process if the first writer, so it writes header. Then it writes line of data and unlock file.
> 
> On the other hand, CSV file is proposed for parsing by scripts. Does such complication really makes sense?
Thanks for the explanation! Yes, it would be more complicated indeed if locking is required. Let's forget the header if it's not practical.

However that raises a question: is it up to the user/build system to delete the log file on startup? Otherwise each build would indefintely append, you wouldn't know where your last build started in the log? How do you address that currently?


================
Comment at: clang/docs/UsersManual.rst:787
+  $ clang -fproc-stat-report=- foo.c  
+  clang-11: output=/tmp/foo-123456.o, total=84000, user=76000, mem=87496
+  ld: output=a.out, total=8000, user=8000, mem=53548
----------------
sepavloff wrote:
> aganea wrote:
> > MaskRay wrote:
> > > aganea wrote:
> > > > I think it is better if the units are specified along (and locale-formatted, if possible):
> > > > ```
> > > > clang-11: output=/tmp/foo-123456.o  total=84,000 ms  user=76,000 ms  mem=87,496 kb
> > > > ```
> > > Sorry, I tend to disagree with the argument for decimal separators and locale differences. They make behaviors divergent and make the output difficult to parse by a script. (Scripts may have to use `LANG=C clang -fproc-stat-report=-` to cancel the locale effect)
> > In my sense, CSV or YAML are for machine parsing, TXT is for human consumption. A script should not parse a human-targetted output.
> > Maybe an option is missing here to set the format? Again xray has `-f=text` or `-f=csv`.
> I don't think an option for output format is needed here (but it could be useful if we supported JSON in addition to CSV). However the proposed format indeed is more readable. I am going to change printing accordingly, but without locale specifics. For users of this feature C locale is convenient.
Sounds good!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78903/new/

https://reviews.llvm.org/D78903