[clang] b00de4d - [clang][deps] Abolish FileManager sharing

Jan Svoboda via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 4 12:47:34 PDT 2022


Author: Jan Svoboda
Date: 2022-10-04T12:47:22-07:00
New Revision: b00de4dd4156874fd5c163e9cecd69a54e45e083

URL: https://github.com/llvm/llvm-project/commit/b00de4dd4156874fd5c163e9cecd69a54e45e083
DIFF: https://github.com/llvm/llvm-project/commit/b00de4dd4156874fd5c163e9cecd69a54e45e083.diff

LOG: [clang][deps] Abolish FileManager sharing

This patch removes the ability of a dependency scanning worker to share a `FileManager` instance between individual scans. It's not sound and doesn't provide performance benefits (due to the underlying caching VFS).

Reviewed By: benlangmuir

Differential Revision: https://reviews.llvm.org/D134976

Added: 
    

Modified: 
    clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
    clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
    clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
    clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
    clang/test/ClangScanDeps/modulemap-via-vfs.m
    clang/test/ClangScanDeps/subframework_header_dir_symlink.m
    clang/test/ClangScanDeps/symlink.cpp
    clang/tools/clang-scan-deps/ClangScanDeps.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
index 8bb983d9e3d55..a8cb15847b781 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
@@ -40,12 +40,11 @@ enum class ScanningOutputFormat {
   Full,
 };
 
-/// The dependency scanning service contains the shared state that is used by
-/// the invidual dependency scanning workers.
+/// The dependency scanning service contains shared configuration and state that
+/// is used by the individual dependency scanning workers.
 class DependencyScanningService {
 public:
   DependencyScanningService(ScanningMode Mode, ScanningOutputFormat Format,
-                            bool ReuseFileManager = true,
                             bool OptimizeArgs = false,
                             bool EagerLoadModules = false);
 
@@ -53,8 +52,6 @@ class DependencyScanningService {
 
   ScanningOutputFormat getFormat() const { return Format; }
 
-  bool canReuseFileManager() const { return ReuseFileManager; }
-
   bool canOptimizeArgs() const { return OptimizeArgs; }
 
   bool shouldEagerLoadModules() const { return EagerLoadModules; }
@@ -66,7 +63,6 @@ class DependencyScanningService {
 private:
   const ScanningMode Mode;
   const ScanningOutputFormat Format;
-  const bool ReuseFileManager;
   /// Whether to optimize the modules' command-line arguments.
   const bool OptimizeArgs;
   /// Whether to set up command-lines to load PCM files eagerly.

diff  --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
index 4750c9dfa9a21..c5776acadcb93 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -100,9 +100,6 @@ class DependencyScanningWorker {
   /// dependencies. This filesystem persists across multiple compiler
   /// invocations.
   llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS;
-  /// The file manager that is reused across multiple invocations by this
-  /// worker. If null, the file manager will not be reused.
-  llvm::IntrusiveRefCntPtr<FileManager> Files;
   ScanningOutputFormat Format;
   /// Whether to optimize the modules' command-line arguments.
   bool OptimizeArgs;

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
index 913524a2e2114..6dccb3d131cd7 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
@@ -14,10 +14,10 @@ using namespace tooling;
 using namespace dependencies;
 
 DependencyScanningService::DependencyScanningService(
-    ScanningMode Mode, ScanningOutputFormat Format, bool ReuseFileManager,
-    bool OptimizeArgs, bool EagerLoadModules)
-    : Mode(Mode), Format(Format), ReuseFileManager(ReuseFileManager),
-      OptimizeArgs(OptimizeArgs), EagerLoadModules(EagerLoadModules) {
+    ScanningMode Mode, ScanningOutputFormat Format, bool OptimizeArgs,
+    bool EagerLoadModules)
+    : Mode(Mode), Format(Format), OptimizeArgs(OptimizeArgs),
+      EagerLoadModules(EagerLoadModules) {
   // Initialize targets for object file support.
   llvm::InitializeAllTargets();
   llvm::InitializeAllTargetMCs();

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 329eded0e3434..33dc5797b77db 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -270,8 +270,6 @@ class DependencyScanningAction : public tooling::ToolAction {
       Action = std::make_unique<ReadPCHAndPreprocessAction>();
 
     const bool Result = ScanInstance.ExecuteAction(*Action);
-    if (!DepFS)
-      FileMgr->clearStatCache();
 
     if (Result)
       setLastCC1Arguments(std::move(OriginalInvocation));
@@ -336,8 +334,6 @@ DependencyScanningWorker::DependencyScanningWorker(
   if (Service.getMode() == ScanningMode::DependencyDirectivesScan)
     DepFS = new DependencyScanningWorkerFilesystem(Service.getSharedCache(),
                                                    RealFS);
-  if (Service.canReuseFileManager())
-    Files = new FileManager(FileSystemOptions(), RealFS);
 }
 
 llvm::Error DependencyScanningWorker::computeDependencies(
@@ -398,11 +394,9 @@ bool DependencyScanningWorker::computeDependencies(
     llvm::Optional<StringRef> ModuleName) {
   // Reset what might have been modified in the previous worker invocation.
   RealFS->setCurrentWorkingDirectory(WorkingDirectory);
-  if (Files)
-    Files->setVirtualFileSystem(RealFS);
 
-  llvm::IntrusiveRefCntPtr<FileManager> CurrentFiles =
-      Files ? Files : new FileManager(FileSystemOptions(), RealFS);
+  auto FileMgr =
+      llvm::makeIntrusiveRefCnt<FileManager>(FileSystemOptions(), RealFS);
 
   Optional<std::vector<std::string>> ModifiedCommandLine;
   if (ModuleName) {
@@ -426,7 +420,7 @@ bool DependencyScanningWorker::computeDependencies(
 
   // Although `Diagnostics` are used only for command-line parsing, the
   // custom `DiagConsumer` might expect a `SourceManager` to be present.
-  SourceManager SrcMgr(*Diags, *CurrentFiles);
+  SourceManager SrcMgr(*Diags, *FileMgr);
   Diags->setSourceManager(&SrcMgr);
   // DisableFree is modified by Tooling for running
   // in-process; preserve the original value, which is
@@ -436,7 +430,7 @@ bool DependencyScanningWorker::computeDependencies(
                                   OptimizeArgs, EagerLoadModules, DisableFree,
                                   ModuleName);
   bool Success = forEachDriverJob(
-      FinalCommandLine, *Diags, *CurrentFiles, [&](const driver::Command &Cmd) {
+      FinalCommandLine, *Diags, *FileMgr, [&](const driver::Command &Cmd) {
         if (StringRef(Cmd.getCreator().getName()) != "clang") {
           // Non-clang command. Just pass through to the dependency
           // consumer.
@@ -455,7 +449,7 @@ bool DependencyScanningWorker::computeDependencies(
         // system to ensure that any file system requests that
         // are made by the driver do not go through the
         // dependency scanning filesystem.
-        ToolInvocation Invocation(std::move(Argv), &Action, &*CurrentFiles,
+        ToolInvocation Invocation(std::move(Argv), &Action, &*FileMgr,
                                   PCHContainerOps);
         Invocation.setDiagnosticConsumer(Diags->getClient());
         Invocation.setDiagnosticOptions(&Diags->getDiagnosticOptions());

diff  --git a/clang/test/ClangScanDeps/modulemap-via-vfs.m b/clang/test/ClangScanDeps/modulemap-via-vfs.m
index 6eb7df725a686..e2db23cd9e861 100644
--- a/clang/test/ClangScanDeps/modulemap-via-vfs.m
+++ b/clang/test/ClangScanDeps/modulemap-via-vfs.m
@@ -3,8 +3,7 @@
 // RUN: sed -e "s|DIR|%/t.dir|g" %t.dir/build/compile-commands.json.in > %t.dir/build/compile-commands.json
 // RUN: sed -e "s|DIR|%/t.dir|g" %t.dir/build/vfs.yaml.in > %t.dir/build/vfs.yaml
 // RUN: clang-scan-deps -compilation-database %t.dir/build/compile-commands.json \
-// RUN:   -reuse-filemanager=0 -j 1 -format experimental-full \
-// RUN:   -mode preprocess-dependency-directives > %t.db
+// RUN:   -j 1 -format experimental-full -mode preprocess-dependency-directives > %t.db
 // RUN: %deps-to-rsp %t.db --module-name=A > %t.A.cc1.rsp
 // RUN: cat %t.A.cc1.rsp | sed 's:\\\\\?:/:g' | FileCheck %s
 

diff  --git a/clang/test/ClangScanDeps/subframework_header_dir_symlink.m b/clang/test/ClangScanDeps/subframework_header_dir_symlink.m
index 46f8f5bb0c522..3bbc5320d4e0c 100644
--- a/clang/test/ClangScanDeps/subframework_header_dir_symlink.m
+++ b/clang/test/ClangScanDeps/subframework_header_dir_symlink.m
@@ -8,10 +8,7 @@
 // RUN: cp -R %S/Inputs/frameworks %t.dir/Inputs/frameworks
 // RUN: ln -s %t.dir/Inputs/frameworks %t.dir/Inputs/frameworks_symlink
 // RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/subframework_header_dir_symlink_cdb.json > %t.cdb
-// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -reuse-filemanager=0 | \
-// RUN:   FileCheck %s
-// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -reuse-filemanager=1 | \
-// RUN:   FileCheck %s
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 |  FileCheck %s
 
 #ifndef EMPTY
 #include "Framework/Framework.h"

diff  --git a/clang/test/ClangScanDeps/symlink.cpp b/clang/test/ClangScanDeps/symlink.cpp
index a4394c0da2a72..d262f8c7f1d95 100644
--- a/clang/test/ClangScanDeps/symlink.cpp
+++ b/clang/test/ClangScanDeps/symlink.cpp
@@ -8,8 +8,7 @@
 // RUN: cp %S/Inputs/header.h %t.dir/Inputs/header.h
 // RUN: ln -s %t.dir/Inputs/header.h %t.dir/Inputs/symlink.h
 // RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/symlink_cdb.json > %t.cdb
-// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -reuse-filemanager=0 | FileCheck %s
-// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -reuse-filemanager=1 | FileCheck %s
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | FileCheck %s
 
 #include "symlink.h"
 #include "header.h"

diff  --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index 28da64f9992e3..0b91234e1083e 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -167,11 +167,6 @@ llvm::cl::opt<std::string>
                   llvm::cl::desc("Compilation database"), llvm::cl::Required,
                   llvm::cl::cat(DependencyScannerCategory));
 
-llvm::cl::opt<bool> ReuseFileManager(
-    "reuse-filemanager",
-    llvm::cl::desc("Reuse the file manager and its cache between invocations."),
-    llvm::cl::init(true), llvm::cl::cat(DependencyScannerCategory));
-
 llvm::cl::opt<std::string> ModuleName(
     "module-name", llvm::cl::Optional,
     llvm::cl::desc("the module of which the dependencies are to be computed"),
@@ -529,8 +524,8 @@ int main(int argc, const char **argv) {
   // Print out the dependency results to STDOUT by default.
   SharedStream DependencyOS(llvm::outs());
 
-  DependencyScanningService Service(ScanMode, Format, ReuseFileManager,
-                                    OptimizeArgs, EagerLoadModules);
+  DependencyScanningService Service(ScanMode, Format, OptimizeArgs,
+                                    EagerLoadModules);
   llvm::ThreadPool Pool(llvm::hardware_concurrency(NumThreads));
   std::vector<std::unique_ptr<DependencyScanningTool>> WorkerTools;
   for (unsigned I = 0; I < Pool.getThreadCount(); ++I)


        


More information about the cfe-commits mailing list