[PATCH] D63681: [clang-scan-deps] Introduce the DependencyScanning library with the thread worker code and better error handling

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364474: [clang-scan-deps] Introduce the DependencyScanning 
library with the (authored by arphaman, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63681?vs=206100=206744#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63681

Files:
  cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
  cfe/trunk/include/clang/Tooling/Tooling.h
  cfe/trunk/lib/Tooling/CMakeLists.txt
  cfe/trunk/lib/Tooling/DependencyScanning/CMakeLists.txt
  cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  cfe/trunk/lib/Tooling/Tooling.cpp
  cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json
  cfe/trunk/test/ClangScanDeps/error.cpp
  cfe/trunk/tools/clang-scan-deps/CMakeLists.txt
  cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp

Index: cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json
===
--- cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json
+++ cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json
@@ -6,7 +6,7 @@
 },
 {
   "directory": "DIR",
-  "command": "clang -E -fsyntax-only DIR/regular_cdb.cpp -IInputs",
+  "command": "clang -E DIR/regular_cdb.cpp -IInputs",
   "file": "DIR/regular_cdb.cpp"
 }
 ]
Index: cfe/trunk/test/ClangScanDeps/error.cpp
===
--- cfe/trunk/test/ClangScanDeps/error.cpp
+++ cfe/trunk/test/ClangScanDeps/error.cpp
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/regular_cdb.cpp
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/regular_cdb.json > %t.cdb
+//
+// RUN: not clang-scan-deps -compilation-database %t.cdb -j 1 2>%t.dir/errs
+// RUN: echo EOF >> %t.dir/errs
+// RUN: FileCheck %s --input-file %t.dir/errs
+
+#include "missing.h"
+
+// CHECK: Error while scanning dependencies
+// CHECK-NEXT: error: no such file or directory:
+// CHECK-NEXT: error: no input files
+// CHECK-NEXT: error:
+// CHECK-NEXT: Error while scanning dependencies
+// CHECK-NEXT: fatal error: 'missing.h' file not found
+// CHECK-NEXT: "missing.h"
+// CHECK-NEXT: ^
+// CHECK-NEXT: EOF
Index: cfe/trunk/lib/Tooling/DependencyScanning/CMakeLists.txt
===
--- cfe/trunk/lib/Tooling/DependencyScanning/CMakeLists.txt
+++ cfe/trunk/lib/Tooling/DependencyScanning/CMakeLists.txt
@@ -0,0 +1,22 @@
+set(LLVM_LINK_COMPONENTS
+  Core
+  Support
+  )
+
+add_clang_library(clangDependencyScanning
+  DependencyScanningWorker.cpp
+
+  DEPENDS
+  ClangDriverOptions
+
+  LINK_LIBS
+  clangAST
+  clangBasic
+  clangDriver
+  clangFrontend
+  clangFrontendTool
+  clangLex
+  clangParse
+  clangSerialization
+  clangTooling
+)
Index: cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===
--- cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -0,0 +1,149 @@
+//===- DependencyScanningWorker.cpp - clang-scan-deps worker --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Tooling/Tooling.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace dependencies;
+
+namespace {
+
+/// Prints out all of the gathered dependencies into a string.
+class DependencyPrinter : public DependencyFileGenerator {
+public:
+  DependencyPrinter(std::unique_ptr Opts,
+std::string )
+  : DependencyFileGenerator(*Opts), Opts(std::move(Opts)), S(S) {}
+
+  void finishedMainFile(DiagnosticsEngine ) override {
+llvm::raw_string_ostream OS(S);
+outputDependencyFile(OS);
+  }
+
+private:
+  std::unique_ptr Opts;
+  std::string 
+};
+
+/// A proxy file system that doesn't call `chdir` when changing the working
+/// directory of a clang tool.
+class ProxyFileSystemWithoutChdir : public llvm::vfs::ProxyFileSystem {
+public:
+  ProxyFileSystemWithoutChdir(
+  llvm::IntrusiveRefCntPtr FS)
+  : ProxyFileSystem(std::move(FS)) {}
+
+  llvm::ErrorOr getCurrentWorkingDirectory() const override {
+assert(!CWD.empty() && "empty CWD");
+return CWD;
+  }
+
+  std::error_code 

[PATCH] D63681: [clang-scan-deps] Introduce the DependencyScanning library with the thread worker code and better error handling

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

@aganea The FS `status` and most of file reads will be cached with the shared 
FS. Hopefully I can put up a patch for it this week.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63681



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63681: [clang-scan-deps] Introduce the DependencyScanning library with the thread worker code and better error handling

[Alex Lorenz via Phabricator via cfe-commits]

arphaman marked an inline comment as done.
arphaman added inline comments.



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h:29
+/// sources either using a fast mode where the source files are minimized, or
+/// using the regular processing run.
+class DependencyScanningWorker {

aganea wrote:
> How do you actually select between the fast mode or the regular preprocess?
You can't select it yet, as it's not yet implemented :)
A couple more follow-up patches, and it will be usable.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63681



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63681: [clang-scan-deps] Introduce the DependencyScanning library with the thread worker code and better error handling

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a subscriber: rnk.
aganea added a comment.

A bit more detail on what we're seeing on our end (specs in the post above). 
The 'Count' column represents the number of 1ms samples taken in that function. 
The 'Weight' column is cumulated times for all cores, for a given process, in 
ms.

Image below shows most of the time is spent in `clang-scan-deps.dll` and 
indirectly, in `ntoskrnl.exe`. The graph at the bottom also shows that 
preprocessing the ~600 unity .CPP files in the project issues 16 million IOPS. 
The graphs on the right show no bottlenecks on the disk I/O on the OS I/O-side.

F9417850: clang-scan-deps.jpg 

Top functions in `ntoskrnl.exe`, most callstacks end up in 
`clang-scan-deps.dll`:

F9417863: clang-scan-deps2.jpg 

A good chunk of all this is caused by `llvm::sys::status()`, even though is 
goes through the `FileSystemStatCache`, like I was suggesting previously 
 (ping @rnk)
//(beware, the callstacks are reversed in the image below)//

F9417870: clang-status.JPG 

I can take a look at `llvm::sys::status()` after the vacations (somewhere in 
August)


Repository:
  rC Clang

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

https://reviews.llvm.org/D63681



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63681: [clang-scan-deps] Introduce the DependencyScanning library with the thread worker code and better error handling

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

LGTM!

Some quick stats on our end (running Windows 10 on a Intel W-2135, 6-core, 3.7 
GHz, NVMe SSD): on a large .SLN compiling approx. 16,000 .CPP files through 600 
unity .CPPs and 23,000 .H files, out of **86 secs** spent in `ClangScanDeps`, 
about **32 secs** are spent in `DirectoryLookup::LookUpFile`.




Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h:29
+/// sources either using a fast mode where the source files are minimized, or
+/// using the regular processing run.
+class DependencyScanningWorker {

How do you actually select between the fast mode or the regular preprocess?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63681



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63681: [clang-scan-deps] Introduce the DependencyScanning library with the thread worker code and better error handling

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added reviewers: Bigcheese, aganea.
Herald added subscribers: tschuett, dexonsmith, jkorous, mgorny.
Herald added a project: clang.

This patch extracts out the code that will powers the fast scanning worker into 
a new file in a new DependencyScanning library. The error and output handling 
is improved so that the clients can gather errors/results from the worker 
directly.


Repository:
  rC Clang

https://reviews.llvm.org/D63681

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
  clang/include/clang/Tooling/Tooling.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/DependencyScanning/CMakeLists.txt
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/test/ClangScanDeps/Inputs/regular_cdb.json
  clang/test/ClangScanDeps/error.cpp
  clang/tools/clang-scan-deps/CMakeLists.txt
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -7,18 +7,11 @@
 //===--===//
 
 #include "clang/Frontend/CompilerInstance.h"
-#include "clang/Frontend/CompilerInvocation.h"
-#include "clang/Frontend/FrontendActions.h"
-#include "clang/Frontend/PCHContainerOperations.h"
-#include "clang/FrontendTool/Utils.h"
 #include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h"
 #include "clang/Tooling/JSONCompilationDatabase.h"
-#include "clang/Tooling/Tooling.h"
-#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/InitLLVM.h"
-#include "llvm/Support/JSON.h"
 #include "llvm/Support/Options.h"
-#include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/Threading.h"
@@ -26,6 +19,7 @@
 #include 
 
 using namespace clang;
+using namespace tooling::dependencies;
 
 namespace {
 
@@ -43,95 +37,6 @@
   raw_ostream 
 };
 
-/// Prints out all of the gathered dependencies into one output stream instead
-/// of using the output dependency file.
-class DependencyPrinter : public DependencyFileGenerator {
-public:
-  DependencyPrinter(std::unique_ptr Opts,
-SharedStream )
-  : DependencyFileGenerator(*Opts), Opts(std::move(Opts)), OS(OS) {}
-
-  void finishedMainFile(DiagnosticsEngine ) override {
-OS.applyLocked([this](raw_ostream ) { outputDependencyFile(OS); });
-  }
-
-private:
-  std::unique_ptr Opts;
-  SharedStream 
-};
-
-/// A clang tool that runs the preprocessor only for the given compiler
-/// invocation.
-class PreprocessorOnlyTool : public tooling::ToolAction {
-public:
-  PreprocessorOnlyTool(StringRef WorkingDirectory, SharedStream )
-  : WorkingDirectory(WorkingDirectory), OS(OS) {}
-
-  bool runInvocation(std::shared_ptr Invocation,
- FileManager *FileMgr,
- std::shared_ptr PCHContainerOps,
- DiagnosticConsumer *DiagConsumer) override {
-// Create a compiler instance to handle the actual work.
-CompilerInstance Compiler(std::move(PCHContainerOps));
-Compiler.setInvocation(std::move(Invocation));
-FileMgr->getFileSystemOpts().WorkingDir = WorkingDirectory;
-Compiler.setFileManager(FileMgr);
-
-// Create the compiler's actual diagnostics engine.
-Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
-if (!Compiler.hasDiagnostics())
-  return false;
-
-Compiler.createSourceManager(*FileMgr);
-
-// Create the dependency collector that will collect the produced
-// dependencies.
-//
-// This also moves the existing dependency output options from the
-// invocation to the collector. The options in the invocation are reset,
-// which ensures that the compiler won't create new dependency collectors,
-// and thus won't write out the extra '.d' files to disk.
-auto Opts = llvm::make_unique(
-std::move(Compiler.getInvocation().getDependencyOutputOpts()));
-// We need at least one -MT equivalent for the generator to work.
-if (Opts->Targets.empty())
-  Opts->Targets = {"clang-scan-deps dependency"};
-Compiler.addDependencyCollector(
-std::make_shared(std::move(Opts), OS));
-
-auto Action = llvm::make_unique();
-const bool Result = Compiler.ExecuteAction(*Action);
-FileMgr->clearStatCache();
-return Result;
-  }
-
-private:
-  StringRef WorkingDirectory;
-  SharedStream 
-};
-
-/// A proxy file system that doesn't call `chdir` when changing the working
-/// directory of a clang tool.
-class ProxyFileSystemWithoutChdir : public llvm::vfs::ProxyFileSystem {
-public:
-  ProxyFileSystemWithoutChdir(
-  llvm::IntrusiveRefCntPtr FS)
-  : ProxyFileSystem(std::move(FS)) {}
-
-