Skip to content

Commit

Permalink
[clang][deps] Abolish FileManager sharing
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jansvoboda11 committed Oct 4, 2022
1 parent e212a4f commit b00de4d
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 39 deletions.
Expand Up @@ -40,21 +40,18 @@ 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);

ScanningMode getMode() const { return Mode; }

ScanningOutputFormat getFormat() const { return Format; }

bool canReuseFileManager() const { return ReuseFileManager; }

bool canOptimizeArgs() const { return OptimizeArgs; }

bool shouldEagerLoadModules() const { return EagerLoadModules; }
Expand All @@ -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.
Expand Down
Expand Up @@ -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;
Expand Down
Expand Up @@ -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();
Expand Down
16 changes: 5 additions & 11 deletions clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
Expand Up @@ -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));
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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());
Expand Down
3 changes: 1 addition & 2 deletions clang/test/ClangScanDeps/modulemap-via-vfs.m
Expand Up @@ -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

Expand Down
5 changes: 1 addition & 4 deletions clang/test/ClangScanDeps/subframework_header_dir_symlink.m
Expand Up @@ -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"
Expand Down
3 changes: 1 addition & 2 deletions clang/test/ClangScanDeps/symlink.cpp
Expand Up @@ -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"
Expand Down
9 changes: 2 additions & 7 deletions clang/tools/clang-scan-deps/ClangScanDeps.cpp
Expand Up @@ -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"),
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit b00de4d

Please sign in to comment.