Speeding up llvm-cov export with multithreaded renderFiles implementation.

[sajjadm]

Summary

CoverageExporterJson::renderFiles accounts for most of the execution time given a large profdata file with multiple binaries.

Proposed solution is to generate JSON for each file in parallel and sort at the end to preserve deterministic output. Also added flags to skip generating parts of the output to trim the output size.

Patch by Sajjad Mirza (@sajjadm).

[Dor1s]

Thanks for the patch, @sajjadm ! I left some comments. You would also need to update the documentation page and add a test for the new behavior you're introducing.

llvm/tools/llvm-cov/CoverageExporterJson.cpp

137 ↗

(On Diff #190352)

Maybe we should pass the whole ViewOpts object here now, rather than individual flags? Same for renderFiles

197 ↗

(On Diff #190352)

It's unclear what exactly this function returns. From the declaration it feels like it will return true for equal Json objects and false otherwise. However, it doesn't seem to be the case. Could you please rename it to be more explicit?

218 ↗

(On Diff #190352)

Let's pass the whole Options object and create the Pool inside renderFiles.

223 ↗

(On Diff #190352)

update the comment to something like "Skip functions-level information if necessary".

[liaoyuke]

Have we written tests for the new behaviors?

[vsk]

llvm/tools/llvm-cov/CoverageExporterJson.cpp

220 ↗

(On Diff #190352)

As this is the only use of the comparator, consider passing in a lambda which consumes const json::Object & parameters.

[Dor1s]

In D59277#1427694, @liaoyuke wrote:

Have we written tests for the new behaviors?

Not sure who this question was addressed to, but yeah we've been always adding/modifying tests when changing something, e.g. adding a new flag to llvm-cov export (https://reviews.llvm.org/D41085) or your fix for symlink issue (https://reviews.llvm.org/D44960).

[liaoyuke]

In D59277#1427791, @Dor1s wrote:

In D59277#1427694, @liaoyuke wrote:

Have we written tests for the new behaviors?

Not sure who this question was addressed to, but yeah we've been always adding/modifying tests when changing something, e.g. adding a new flag to llvm-cov export (https://reviews.llvm.org/D41085) or your fix for symlink issue (https://reviews.llvm.org/D44960).

Sorry for the confusion, it was to sajjadm@ for this change.

[sajjadm]

• Added tests for the -skip-* flags.
• Minor changes in response to code review.

[sajjadm]

Thanks for all the feedback, Max and Vedant!

@Dor1s , do you still find the comparison function confusing now that it's a lambda? It's meant to return a boolean in the manner expected by std::sort. True if the left argument is "less" than the right argument, and false otherwise.

[vsk]

Thanks for working on this!

llvm/tools/llvm-cov/CoverageExporterJson.cpp

158 ↗

(On Diff #190531)

Can the parallelism be controlled by -num-threads/-j, similar to: https://llvm.org/docs/CommandGuide/llvm-cov.html#cmdoption-llvm-cov-show-num-threads

This will help write a test that the output from e.g. -j 4 matches the output from -j 1.

[sajjadm]

llvm/tools/llvm-cov/CoverageExporterJson.cpp

158 ↗

(On Diff #190531)

Yes, this code respects -num-threads. There's actually a test that already does the check you're describing: multithreaded-report.test.

[sajjadm]

• Added docs for new llvm-cov flags.

[sajjadm]

• Changed how -num-threads is documented.

[sajjadm]

I was debating whether I should pull -num-threads out of the docs for show and move it to the top-level options section, or make a copy of it in the export options section. I went with putting a copy of it in export because only show and export use it and the behavior is subtly different (show only uses it if -output-dir is set, export uses it unconditionally).

[Dor1s]

In D59277#1428472, @sajjadm wrote:

Thanks for all the feedback, Max and Vedant!

@Dor1s , do you still find the comparison function confusing now that it's a lambda? It's meant to return a boolean in the manner expected by std::sort. True if the left argument is "less" than the right argument, and false otherwise.

As lambda comparator it totally makes sense.

The change looks good to me! Let's wait for a final approval by @vsk and then I can commit it for you.

[vsk]

Thanks, lgtm.

[Dor1s]

Verified that the tests are passing, getting ready to commit.

docs/CommandGuide/llvm-cov.rst

  will not export coverage information for smaller units such as individual
  functions or regions. The result will contain the same information as produced
  by the :program:`llvm-cov report` command, but presented in JSON or lcov
  format rather than text.
 
 .. option:: -ignore-filename-regex=<PATTERN>
 
  Skip source code files with file paths that match the given regular expression.
+
+ .. option:: -skip-expansions
+
+ Skip exporting macro expansion coverage data.
+
+ .. option:: -skip-functions
+
+ Skip exporting per-function coverage data.
+
+ .. option:: -num-threads=N, -j=N
+
+ Use N threads to export coverage data. When N=0, llvm-cov auto-detects an
+ appropriate number of threads to use. This is the default.

test/tools/llvm-cov/export_functions.test

+# Test that llvm-cov export produces function data by default and that it can be
+# turned off with a flag.
+
+RUN: llvm-cov export %S/Inputs/report.covmapping -instr-profile %S/Inputs/report.profdata 2>&1 | FileCheck %s
+RUN: llvm-cov export %S/Inputs/report.covmapping -instr-profile %S/Inputs/report.profdata -skip-functions 2>&1 | FileCheck -check-prefix=SKIP-FUNCTIONS %s
+
+CHECK: "functions":[
+SKIP-FUNCTIONS-NOT: "functions":[
+
+

test/tools/llvm-cov/showExpansions.cpp

 // CHECK-DAG: Expansion at line [[@LINE-3]], 7 -> 20
 
 int main(int argc, const char *argv[]) {
   for (int i = 0; i < 100; ++i)
     DO_SOMETHING(i); // CHECK-DAG: Expansion at line [[@LINE]], 5 -> 17
   return 0;
 }
 // RUN: llvm-cov export %S/Inputs/showExpansions.covmapping -instr-profile %S/Inputs/showExpansions.profdata 2>&1 | FileCheck %S/Inputs/showExpansions.json
+
+// RUN: llvm-cov export %S/Inputs/showExpansions.covmapping -instr-profile %S/Inputs/showExpansions.profdata -skip-expansions 2>&1 | FileCheck %s -check-prefix=SKIP-EXPANSIONS
+// SKIP-EXPANSIONS-NOT: "expansions"

tools/llvm-cov/CodeCoverage.cpp

if (!ShowFunctionSummaries) {
     Report.renderFunctionReports(SourceFiles, DC, llvm::outs());
   }
   return 0;
 }
 
 int CodeCoverageTool::doExport(int argc, const char **argv,
                                CommandLineParserType commandLineParser) {
 
+  cl::OptionCategory ExportCategory("Exporting options");
+
+  cl::opt<bool> SkipExpansions("skip-expansions", cl::Optional,
+                               cl::desc("Don't export expanded source regions"),
+                               cl::cat(ExportCategory));
+
+  cl::opt<bool> SkipFunctions("skip-functions", cl::Optional,
+                              cl::desc("Don't export per-function data"),
+                              cl::cat(ExportCategory));
+
   auto Err = commandLineParser(argc, argv);
   if (Err)
     return Err;
 
+  ViewOpts.SkipExpansions = SkipExpansions;
+  ViewOpts.SkipFunctions = SkipFunctions;
+
   if (ViewOpts.Format != CoverageViewOptions::OutputFormat::Text &&
       ViewOpts.Format != CoverageViewOptions::OutputFormat::Lcov) {
     error("Coverage data can only be exported as textual JSON or an "
           "lcov tracefile.");
     return 1;
   }
 
   auto Coverage = load();

tools/llvm-cov/CoverageExporterJson.cpp

 //     -- FunctionCoverage: dict => Object summarizing function coverage
 //     -- InstantiationCoverage: dict => Object summarizing inst. coverage
 //     -- RegionCoverage: dict => Object summarizing region coverage
 //
 //===----------------------------------------------------------------------===//
 
 #include "CoverageExporterJson.h"
 #include "CoverageReport.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/JSON.h"
+#include "llvm/Support/ThreadPool.h"
+#include "llvm/Support/Threading.h"
+#include <algorithm>
+#include <mutex>
+#include <utility>
 
 /// The semantic version combined as a string.
 #define LLVM_COVERAGE_EXPORT_JSON_STR "2.0.0"
 
 /// Unique type identifier for JSON coverage export.
 #define LLVM_COVERAGE_EXPORT_JSON_TYPE_STR "llvm.coverage.json.export"
 
 using namespace llvm;
json::Array renderFileSegments(const coverage::CoverageData &FileCoverage,
   for (const auto &Segment : FileCoverage)
     SegmentArray.push_back(renderSegment(Segment));
   return SegmentArray;
 }
 
 json::Object renderFile(const coverage::CoverageMapping &Coverage,
                         const std::string &Filename,
                         const FileCoverageSummary &FileReport,
-                        bool ExportSummaryOnly) {
+                        const CoverageViewOptions &Options) {
   json::Object File({{"filename", Filename}});
-  if (!ExportSummaryOnly) {
+  if (!Options.ExportSummaryOnly) {
     // Calculate and render detailed coverage information for given file.
     auto FileCoverage = Coverage.getCoverageForFile(Filename);
     File["segments"] = renderFileSegments(FileCoverage, FileReport);
+    if (!Options.SkipExpansions) {
       File["expansions"] = renderFileExpansions(FileCoverage, FileReport);
     }
+  }
   File["summary"] = renderSummary(FileReport);
   return File;
 }
 
 json::Array renderFiles(const coverage::CoverageMapping &Coverage,
                         ArrayRef<std::string> SourceFiles,
                         ArrayRef<FileCoverageSummary> FileReports,
-                        bool ExportSummaryOnly) {
+                        const CoverageViewOptions &Options) {
+  auto NumThreads = Options.NumThreads;
+  if (NumThreads == 0) {
+    NumThreads = std::max(1U, std::min(llvm::heavyweight_hardware_concurrency(),
+                                       unsigned(SourceFiles.size())));
+  }
+  ThreadPool Pool(NumThreads);
   json::Array FileArray;
-  for (unsigned I = 0, E = SourceFiles.size(); I < E; ++I)
-    FileArray.push_back(renderFile(Coverage, SourceFiles[I], FileReports[I],
-                                   ExportSummaryOnly));
+  std::mutex FileArrayMutex;
+
+  for (unsigned I = 0, E = SourceFiles.size(); I < E; ++I) {
+    auto &SourceFile = SourceFiles[I];
+    auto &FileReport = FileReports[I];
+    Pool.async([&] {
+      auto File = renderFile(Coverage, SourceFile, FileReport, Options);
+      {
+        std::lock_guard<std::mutex> Lock(FileArrayMutex);
+        FileArray.push_back(std::move(File));
+      }
+    });
+  }
+  Pool.wait();
   return FileArray;
 }
 
 json::Array renderFunctions(
     const iterator_range<coverage::FunctionRecordIterator> &Functions) {
   json::Array FunctionArray;
   for (const auto &F : Functions)
     FunctionArray.push_back(
void CoverageExporterJson::renderRoot(const CoverageFilters &IgnoreFilters) {
   }
   renderRoot(SourceFiles);
 }
 
 void CoverageExporterJson::renderRoot(ArrayRef<std::string> SourceFiles) {
   FileCoverageSummary Totals = FileCoverageSummary("Totals");
   auto FileReports = CoverageReport::prepareFileReports(Coverage, Totals,
                                                         SourceFiles, Options);
-  auto Export =
-      json::Object({{"files", renderFiles(Coverage, SourceFiles, FileReports,
-                                          Options.ExportSummaryOnly)},
-                    {"totals", renderSummary(Totals)}});
-  // Skip functions-level information for summary-only export mode.
-  if (!Options.ExportSummaryOnly)
+  auto Files = renderFiles(Coverage, SourceFiles, FileReports, Options);
+  // Sort files in order of their names.
+  std::sort(Files.begin(), Files.end(),
+    [](const json::Value &A, const json::Value &B) {
+      const json::Object *ObjA = A.getAsObject();
+      const json::Object *ObjB = B.getAsObject();
+      assert(ObjA != nullptr && "Value A was not an Object");
+      assert(ObjB != nullptr && "Value B was not an Object");
+      const StringRef FilenameA = ObjA->getString("filename").getValue();
+      const StringRef FilenameB = ObjB->getString("filename").getValue();
+      return FilenameA.compare(FilenameB) < 0;
+    });
+  auto Export = json::Object(
+      {{"files", std::move(Files)}, {"totals", renderSummary(Totals)}});
+  // Skip functions-level information  if necessary.
+  if (!Options.ExportSummaryOnly && !Options.SkipFunctions)
     Export["functions"] = renderFunctions(Coverage.getCoveredFunctions());
 
   auto ExportArray = json::Array({std::move(Export)});
 
   OS << json::Object({{"version", LLVM_COVERAGE_EXPORT_JSON_STR},
                       {"type", LLVM_COVERAGE_EXPORT_JSON_TYPE_STR},
                       {"data", std::move(ExportArray)}});
 }

tools/llvm-cov/CoverageViewOptions.h

struct CoverageViewOptions {
   bool ShowLineStats;
   bool ShowRegionMarkers;
   bool ShowExpandedRegions;
   bool ShowFunctionInstantiations;
   bool ShowFullFilenames;
   bool ShowRegionSummary;
   bool ShowInstantiationSummary;
   bool ExportSummaryOnly;
+  bool SkipExpansions;
+  bool SkipFunctions;
   OutputFormat Format;
   std::string ShowOutputDirectory;
   std::vector<std::string> DemanglerOpts;
   uint32_t TabSize;
   std::string ProjectTitle;
   std::string CreatedTimeStr;
   unsigned NumThreads;