[Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

[vvereschaka]

Summary

Added supporting CC_PRINT_PROC_STAT and CC_PRINT_PROC_STAT_FILE environment variables to trigger clang driver reporting the process statistics into specified file (alternate for -fproc-stat-report option).

[vvereschaka]

@sepavloff would you add more persons in additional to the reviewers?

[vvereschaka]

Updated diff in according of the Lint/clang-format recommendations.

[sepavloff]

clang/include/clang/Driver/Driver.h

210

Strictly speaking it is driver that dumps performance report, not frontend.

[vvereschaka]

Fixed comment.

[vvereschaka]

clang/include/clang/Driver/Driver.h

210

Thanks, I have updated the comment accordingly.

[aganea]

clang/lib/Driver/Driver.cpp

1107

Please remove, getLastArg already claims all arguments for this option.

4031

I think the code below is safe-explanatory, I don't think you need this comment, it doesn't add much value.

4050

The previous behavior was printing both the output and the CSV file when specifying -fproc-stat-report -fproc-stat-report=file.csv, now it would only emit the CSV, is that intended? @sepavloff what do you think?

clang/test/Driver/cc-print-proc-stat.c

1

Could you please expand a bit the test to entirely cover all the codepaths above?

[vvereschaka]

Thank you @aganea

Updated diff:

• removed ClaimAllArgs for OPT_fproc_stat_report_EQ (used in getLastArg);
• used const char* instead of std::string for CCPrintStatReportFilename;
• removed unnecessary comment

[vvereschaka]

clang/lib/Driver/Driver.cpp

4050

now it would only emit the CSV, is that intended?

yes, I thought in that way. I dоn't think that usage of both options at the same time was specially designed. I don't see a reasonable usage case for that for now actually, but I could be wrong. @sepavloff ?

[sepavloff]

clang/lib/Driver/Driver.cpp

4050

now it would only emit the CSV, is that intended?

yes, I thought in that way. I dоn't think that usage of both options at the same time was specially designed. I don't see a reasonable usage case for that for now actually, but I could be wrong. @sepavloff ?

The previous behavior (printing both the output and the CSV file) was not intentional, it was the implementation peculiarity. I don't see real use case for both outputs.

[aganea]

Thanks for the update @vvereschaka ! Just a few minor things and it should be good to go.
Also please use git diff -U99999 next time to add context to the patch.

clang/docs/UsersManual.rst

783

I think this should read "In this case statistics are printed.." Would you mind changing this as well?

799

Do you think it would be possible to rephrase a bit to avoid the repetition of CC_PRINT_PROC_STAT_FILE? Perhaps also explicitate that CC_PRINT_PROC_STAT is for "enabling the feature and logging to stdout"; while CC_PRINT_PROC_STAT_FILE only "redirects the log to a file".

clang/lib/Driver/Driver.cpp

1108

Remove this too, hasArg claims all arguments.

4039

Suggestion: I find if (!CCPrintStatReportFilename) more intutive, more compact and easier to read, but that's my personal preference. There are arguments both ways probably, up to you.

[vvereschaka]

Updated diff:

• rewording the doc part
• removed ClaimAllArgs for OPT_fproc_stat_report
• updated if statement for CCPrintStatReportFilename

[vvereschaka]

clang/docs/UsersManual.rst

799

Yes, sure. Here is updated and extended version of the doc part.

clang/lib/Driver/Driver.cpp

4039

I have updated it in according of you suggestion. It is ok for me too.

[aganea]

LGTM, just a few minor things:

clang/docs/UsersManual.rst

798

"enables the feature, the report goes to the stdout in the human readable format."

800

"the process statistics to the given file in the CSV format"

804

Would you be able to merge this paragraph with the following one please? It could be something like:

"These environment variables are handy when you need to request the statistics report without changing your build scripts or alter the existing set of compiler options. Note that -fproc-stat-report take precedence over CC_PRINT_PROC_STAT and CC_PRINT_PROC_STAT_FILE."

[vvereschaka]

Updated diff:

• updated doc parts

@aganea thanks a lot for your suggestions for doc improvements. I have updated it accordingly.

clang/docs/UsersManual.rst

   is sent to. If it specifies a regular file, the data are saved to this file in
   CSV format:
 
   .. code-block:: console
 
     $ clang -fproc-stat-report=abc foo.c
     $ cat abc
     clang-11,"/tmp/foo-123456.o",92000,84000,87536
     ld,"a.out",900,8000,53568
 
   The data on each row represent:
   

   It is possible to specify this option without any value. In this case statistics
   is printed on standard output in human readable format:
   
   .. code-block:: console
 
     $ clang -fproc-stat-report foo.c
     clang-11: output=/tmp/foo-855a8e.o, total=68.000 ms, user=60.000 ms, mem=86920 Kb
     ld: output=a.out, total=8.000 ms, user=4.000 ms, mem=52320 Kb
   
   The report file specified in the option is locked for write, so this option
   can be used to collect statistics in parallel builds. The report file is not
   cleared, new data is appended to it, thus making posible to accumulate build
   statistics.
 
+  Instead of passing ``-fproc-stat-report`` option it is possible to turn on
+  the process statistic reporting for a whole existing build environment by
+  setting the ``CC_PRINT_PROC_STAT`` and ``CC_PRINT_PROC_STAT_FILE`` environment
+  variables. Use ``CC_PRINT_PROC_STAT_FILE`` to provide a file name to store
+  the statistics.
+
+  .. code-block:: console
+
+    $ export CC_PRINT_PROC_STAT=1
+    $ export CC_PRINT_PROC_STAT_FILE=~/project-build-proc-stat.csv
+    $ make
+
 Other Options
 -------------
 Clang options that don't fit neatly into other categories.

clang/include/clang/Driver/Driver.h

   /// Information about the host which can be overridden by the user.
   std::string HostBits, HostMachine, HostSystem, HostRelease;
 
+  /// The file to log CC_PRINT_PROC_STAT_FILE output to, if enabled.
+  std::string CCPrintStatReportFilename;
+
   /// The file to log CC_PRINT_OPTIONS output to, if enabled.
   const char *CCPrintOptionsFilename;
 

   /// Whether the driver is generating diagnostics for debugging purposes.
   unsigned CCGenDiagnostics : 1;
 
+  /// Set CC_PRINT_PROC_STAT mode, which causes the frontend to dump
+  /// performance report to CC_PRINT_PROC_STAT_FILE or to stdout.
+  unsigned CCPrintProcessStats : 1;
+
   /// Pointer to the ExecuteCC1Tool function, if available.
   /// When the clangDriver lib is used through clang.exe, this provides a
   /// shortcut for executing the -cc1 command-line directly, in the same

clang/lib/Driver/Driver.cpp

       CCPrintHeadersFilename(nullptr), CCLogDiagnosticsFilename(nullptr),
       CCCPrintBindings(false), CCPrintOptions(false), CCPrintHeaders(false),
       CCLogDiagnostics(false), CCGenDiagnostics(false),
-      TargetTriple(TargetTriple), CCCGenericGCCName(""), Saver(Alloc),
-      CheckInputsExist(true), GenReproducer(false),
-      SuppressMissingInputWarning(false) {
+      CCPrintProcessStats(false), TargetTriple(TargetTriple),
+      CCCGenericGCCName(""), Saver(Alloc), CheckInputsExist(true),
+      GenReproducer(false), SuppressMissingInputWarning(false) {
   // Provide a sane fallback if no VFS is specified.
   if (!this->VFS)
     this->VFS = llvm::vfs::getRealFileSystem();

   GenReproducer = Args.hasFlag(options::OPT_gen_reproducer,
                                options::OPT_fno_crash_diagnostics,
                                !!::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH"));
+
+  // Process -fproc-stat-report options.
+  if (const Arg *A = Args.getLastArg(options::OPT_fproc_stat_report_EQ)) {
+    CCPrintProcessStats = true;
+    CCPrintStatReportFilename = A->getValue();
+  }
+  if (Args.hasArg(options::OPT_fproc_stat_report))
+    CCPrintProcessStats = true;
+  Args.ClaimAllArgs(options::OPT_fproc_stat_report);
+  Args.ClaimAllArgs(options::OPT_fproc_stat_report_EQ);
+
   // FIXME: TargetTriple is used by the target-prefixed calls to as/ld
   // and getToolChain is const.
   if (IsCLMode()) {

                        /*TargetDeviceOffloadKind*/ Action::OFK_None);
   }
 
-  StringRef StatReportFile;
-  bool PrintProcessStat = false;
-  if (const Arg *A = C.getArgs().getLastArg(options::OPT_fproc_stat_report_EQ))
-    StatReportFile = A->getValue();
-  if (C.getArgs().hasArg(options::OPT_fproc_stat_report))
-    PrintProcessStat = true;
-
   // If we have more than one job, then disable integrated-cc1 for now. Do this
   // also when we need to report process execution statistics.
-  if (C.getJobs().size() > 1 || !StatReportFile.empty() || PrintProcessStat)
+  if (C.getJobs().size() > 1 || CCPrintProcessStats)
     for (auto &J : C.getJobs())
       J.InProcess = false;
 
-  if (!StatReportFile.empty() || PrintProcessStat) {
+  if (CCPrintProcessStats) {
     C.setPostCallback([=](const Command &Cmd, int Res) {
       Optional<llvm::sys::ProcessStatistics> ProcStat =
           Cmd.getProcessStatistics();
       if (!ProcStat)
         return;
-      if (PrintProcessStat) {
+
+      // Use FinalOutput variable to get the output file name.
+      const char *LinkingOutput = nullptr;
+      if (FinalOutput)
+        LinkingOutput = FinalOutput->getValue();
+      else if (!Cmd.getOutputFilenames().empty())
+        LinkingOutput = Cmd.getOutputFilenames().front().c_str();
+      else
+        LinkingOutput = getDefaultImageName();
+
+      if (CCPrintStatReportFilename.empty()) {
         using namespace llvm;
         // Human readable output.
         outs() << sys::path::filename(Cmd.getExecutable()) << ": "
-               << "output=";
-        if (Cmd.getOutputFilenames().empty())
-          outs() << "\"\"";
-        else
-          outs() << Cmd.getOutputFilenames().front();
+               << "output=" << LinkingOutput;
         outs() << ", total="
                << format("%.3f", ProcStat->TotalTime.count() / 1000.) << " ms"
                << ", user="
                << format("%.3f", ProcStat->UserTime.count() / 1000.) << " ms"
                << ", mem=" << ProcStat->PeakMemory << " Kb\n";
-      }
-      if (!StatReportFile.empty()) {
+      } else {
         // CSV format.
         std::string Buffer;
         llvm::raw_string_ostream Out(Buffer);
         llvm::sys::printArg(Out, llvm::sys::path::filename(Cmd.getExecutable()),
                             /*Quote*/ true);
         Out << ',';
-        if (Cmd.getOutputFilenames().empty())
-          Out << "\"\"";
-        else
-          llvm::sys::printArg(Out, Cmd.getOutputFilenames().front(), true);
+        llvm::sys::printArg(Out, LinkingOutput, true);
         Out << ',' << ProcStat->TotalTime.count() << ','
             << ProcStat->UserTime.count() << ',' << ProcStat->PeakMemory
             << '\n';
         Out.flush();
         std::error_code EC;
-        llvm::raw_fd_ostream OS(StatReportFile, EC, llvm::sys::fs::OF_Append);
+        llvm::raw_fd_ostream OS(CCPrintStatReportFilename, EC,
+                                llvm::sys::fs::OF_Append);
         if (EC)
           return;
         auto L = OS.lock();
         if (!L) {
-          llvm::errs() << "ERROR: Cannot lock file " << StatReportFile << ": "
+          llvm::errs() << "ERROR: Cannot lock file "
+                       << CCPrintStatReportFilename << ": "
                        << toString(L.takeError()) << "\n";
           return;
         }

clang/test/Driver/cc-print-proc-stat.c

+// RUN: env CC_PRINT_PROC_STAT=1 \
+// RUN:     CC_PRINT_PROC_STAT_FILE=%t.csv \
+// RUN: %clang -no-canonical-prefixes -S -o %t.s %s
+// RUN: FileCheck --check-prefix=CSV %s < %t.csv
+
+// CSV: clang{{.*}},"{{.*}}.s",{{[0-9]+}},{{[0-9]+}},{{[0-9]+}}

clang/test/Driver/lit.local.cfg

                              'IPHONEOS_DEPLOYMENT_TARGET',
                              'SDKROOT', 'CCC_OVERRIDE_OPTIONS',
                              'CC_PRINT_OPTIONS', 'CC_PRINT_HEADERS',
-                             'CC_LOG_DIAGNOSTICS']
+                             'CC_LOG_DIAGNOSTICS', 'CC_PRINT_PROC_STAT']
 
 for name in driver_overwrite_env_vars:
   if name in config.environment:

clang/tools/driver/driver.cpp

   TheDriver.CCLogDiagnostics = !!::getenv("CC_LOG_DIAGNOSTICS");
   if (TheDriver.CCLogDiagnostics)
     TheDriver.CCLogDiagnosticsFilename = ::getenv("CC_LOG_DIAGNOSTICS_FILE");
+
+  // Handle CC_PRINT_PROC_STAT and CC_PRINT_PROC_STAT_FILE.
+  TheDriver.CCPrintProcessStats = !!::getenv("CC_PRINT_PROC_STAT");
+  if (TheDriver.CCPrintProcessStats)
+    TheDriver.CCPrintStatReportFilename = ::getenv("CC_PRINT_PROC_STAT_FILE");
 }
 
 static void FixupDiagPrefixExeName(TextDiagnosticPrinter *DiagClient,