From ffa043fb9983462ebeb143afb946c109e9369e7f Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Wed, 6 Feb 2019 17:18:15 +0100 Subject: [PATCH] Clang: Improve performance by reducing parsing We generate one big file per project part so the preprocessor is skipping the recurring includes. This generated many errors about missing macros but we don't care much about them during dependency collection step so we just silence these errors with ignoring diagnostics consumer. Change-Id: I5581d623b5d5f9995496252735577ea6b54790d9 Reviewed-by: Ivan Donchevskii --- .../clangpchmanagerbackendmain.cpp | 4 +- .../source/builddependencycollector.cpp | 48 +++++++++++--- .../source/builddependencycollector.h | 19 +++--- .../source/collectbuilddependencyaction.h | 33 +++++----- ...lectbuilddependencypreprocessorcallbacks.h | 32 +++++++--- .../builddependencycollector-test.cpp | 63 +++++++++++-------- 6 files changed, 131 insertions(+), 68 deletions(-) diff --git a/src/tools/clangpchmanagerbackend/clangpchmanagerbackendmain.cpp b/src/tools/clangpchmanagerbackend/clangpchmanagerbackendmain.cpp index c4e8e085ce2..80bb8886aa8 100644 --- a/src/tools/clangpchmanagerbackend/clangpchmanagerbackendmain.cpp +++ b/src/tools/clangpchmanagerbackend/clangpchmanagerbackendmain.cpp @@ -191,7 +191,9 @@ struct Data // because we have a cycle dependency database}; ClangBackEnd::PchTasksMerger pchTaskMerger{pchTaskQueue}; ClangBackEnd::BuildDependenciesStorage<> buildDependencyStorage{database}; - ClangBackEnd::BuildDependencyCollector buildDependencyCollector{filePathCache, generatedFiles}; + ClangBackEnd::BuildDependencyCollector buildDependencyCollector{filePathCache, + generatedFiles, + environment}; std::function getModifiedTime{ [&](ClangBackEnd::FilePathView path) -> TimeStamp { return QFileInfo(QString(path)).lastModified().toSecsSinceEpoch(); diff --git a/src/tools/clangpchmanagerbackend/source/builddependencycollector.cpp b/src/tools/clangpchmanagerbackend/source/builddependencycollector.cpp index ec5f3bdcb00..b3ebbc75ad7 100644 --- a/src/tools/clangpchmanagerbackend/source/builddependencycollector.cpp +++ b/src/tools/clangpchmanagerbackend/source/builddependencycollector.cpp @@ -28,10 +28,12 @@ #include "collectbuilddependencytoolaction.h" #include "commandlinebuilder.h" +#include + #include #include - +#include namespace ClangBackEnd { namespace { @@ -50,7 +52,7 @@ BuildDependency BuildDependencyCollector::create(const ProjectPartContainer &pro CommandLineBuilder builder{projectPart, projectPart.toolChainArguments, InputFileType::Source}; - addFiles(projectPart.sourcePathIds, builder.commandLine); + addFiles(projectPart.sourcePathIds, std::move(builder.commandLine)); setExcludedFilePaths( m_filePathCache.filePaths(projectPart.headerPathIds + projectPart.sourcePathIds)); @@ -66,6 +68,33 @@ BuildDependency BuildDependencyCollector::create(const ProjectPartContainer &pro return buildDependency; } +namespace { + +std::size_t contentSize(const FilePaths &includes) +{ + auto countIncludeSize = [](std::size_t size, const auto &include) { + return size + include.size(); + }; + + return std::accumulate(includes.begin(), includes.end(), std::size_t(0), countIncludeSize); +} +} // namespace + +Utils::SmallString BuildDependencyCollector::generateFakeFileContent( + const FilePathIds &includeIds) const +{ + Utils::SmallString fileContent; + const std::size_t lineTemplateSize = 12; + auto includes = m_filePathCache.filePaths(includeIds); + + fileContent.reserve(includes.size() * lineTemplateSize + contentSize(includes)); + + for (Utils::SmallStringView include : includes) + fileContent += {"#include \"", include, "\"\n"}; + + return fileContent; +} + void BuildDependencyCollector::collect() { clang::tooling::ClangTool tool = m_clangTool.createTool(); @@ -96,25 +125,26 @@ void BuildDependencyCollector::setExcludedFilePaths(ClangBackEnd::FilePaths &&ex } void BuildDependencyCollector::addFiles(const FilePathIds &filePathIds, - const Utils::SmallStringVector &arguments) + Utils::SmallStringVector &&arguments) { - m_clangTool.addFiles(m_filePathCache.filePaths(filePathIds), arguments); + m_clangTool.addFile(FilePath{m_environment.pchBuildDirectory().toStdString(), "dummy.cpp"}, + generateFakeFileContent(filePathIds), + std::move(arguments)); m_buildDependency.sourceFiles.insert(m_buildDependency.sourceFiles.end(), filePathIds.begin(), filePathIds.end()); } -void BuildDependencyCollector::addFile(FilePathId filePathId, - const Utils::SmallStringVector &arguments) +void BuildDependencyCollector::addFile(FilePathId filePathId, Utils::SmallStringVector &&arguments) { - addFiles({filePathId}, arguments); + addFiles({filePathId}, std::move(arguments)); } void BuildDependencyCollector::addFile(FilePath filePath, const FilePathIds &sourceFileIds, - const Utils::SmallStringVector &arguments) + Utils::SmallStringVector &&arguments) { - m_clangTool.addFiles({filePath}, arguments); + m_clangTool.addFiles({filePath}, std::move(arguments)); m_buildDependency.sourceFiles.insert(m_buildDependency.sourceFiles.end(), sourceFileIds.begin(), sourceFileIds.end()); diff --git a/src/tools/clangpchmanagerbackend/source/builddependencycollector.h b/src/tools/clangpchmanagerbackend/source/builddependencycollector.h index b9446c96928..c5e41e91907 100644 --- a/src/tools/clangpchmanagerbackend/source/builddependencycollector.h +++ b/src/tools/clangpchmanagerbackend/source/builddependencycollector.h @@ -34,33 +34,35 @@ #include namespace ClangBackEnd { +class Environment; class BuildDependencyCollector : public BuildDependencyGeneratorInterface { public: BuildDependencyCollector(const FilePathCachingInterface &filePathCache, - const GeneratedFilesInterface &generatedFiles) + const GeneratedFilesInterface &generatedFiles, + const Environment &environment) : m_filePathCache(filePathCache) , m_generatedFiles(generatedFiles) - { - } + , m_environment(environment) + {} BuildDependency create(const ProjectPartContainer &projectPart) override; void collect(); void setExcludedFilePaths(ClangBackEnd::FilePaths &&excludedIncludes); - void addFiles(const FilePathIds &filePathIds, - const Utils::SmallStringVector &arguments); - void addFile(FilePathId filePathId, - const Utils::SmallStringVector &arguments); + void addFiles(const FilePathIds &filePathIds, Utils::SmallStringVector &&arguments); + void addFile(FilePathId filePathId, Utils::SmallStringVector &&arguments); void addFile(FilePath filePath, const FilePathIds &sourceFileIds, - const Utils::SmallStringVector &arguments); + Utils::SmallStringVector &&arguments); void addUnsavedFiles(const V2::FileContainers &unsavedFiles); void clear(); + Utils::SmallString generateFakeFileContent(const FilePathIds &includeIds) const; + const FileStatuses &fileStatuses() const { return m_buildDependency.fileStatuses; @@ -96,6 +98,7 @@ private: SourcesManager m_sourcesManager; const FilePathCachingInterface &m_filePathCache; const GeneratedFilesInterface &m_generatedFiles; + const Environment &m_environment; }; } // namespace ClangBackEnd diff --git a/src/tools/clangpchmanagerbackend/source/collectbuilddependencyaction.h b/src/tools/clangpchmanagerbackend/source/collectbuilddependencyaction.h index 03d27f9b21e..fa1a10e79c3 100644 --- a/src/tools/clangpchmanagerbackend/source/collectbuilddependencyaction.h +++ b/src/tools/clangpchmanagerbackend/source/collectbuilddependencyaction.h @@ -54,27 +54,28 @@ public: bool BeginSourceFileAction(clang::CompilerInstance &compilerInstance) override { - if (clang::PreprocessOnlyAction::BeginSourceFileAction(compilerInstance)) { - auto &preprocessor = compilerInstance.getPreprocessor(); + if (clang::PreprocessOnlyAction::BeginSourceFileAction(compilerInstance)) { + auto &preprocessor = compilerInstance.getPreprocessor(); - preprocessor.SetSuppressIncludeNotFoundError(true); - preprocessor.SetMacroExpansionOnlyInDirectives(); + preprocessor.SetSuppressIncludeNotFoundError(true); + preprocessor.SetMacroExpansionOnlyInDirectives(); - auto macroPreprocessorCallbacks = new CollectBuildDependencyPreprocessorCallbacks( - m_buildDependency, - m_filePathCache, - m_excludedIncludeUID, - m_alreadyIncludedFileUIDs, - compilerInstance.getSourceManager(), - m_sourcesManager, - compilerInstance.getPreprocessorPtr()); + auto macroPreprocessorCallbacks = new CollectBuildDependencyPreprocessorCallbacks( + m_buildDependency, + m_filePathCache, + m_excludedIncludeUID, + m_alreadyIncludedFileUIDs, + compilerInstance.getSourceManager(), + m_sourcesManager, + compilerInstance.getPreprocessorPtr()); - preprocessor.addPPCallbacks(std::unique_ptr(macroPreprocessorCallbacks)); + preprocessor.addPPCallbacks( + std::unique_ptr(macroPreprocessorCallbacks)); - return true; - } + return true; + } - return false; + return false; } void EndSourceFileAction() override diff --git a/src/tools/clangpchmanagerbackend/source/collectbuilddependencypreprocessorcallbacks.h b/src/tools/clangpchmanagerbackend/source/collectbuilddependencypreprocessorcallbacks.h index 82530927516..980d065fdbc 100644 --- a/src/tools/clangpchmanagerbackend/source/collectbuilddependencypreprocessorcallbacks.h +++ b/src/tools/clangpchmanagerbackend/source/collectbuilddependencypreprocessorcallbacks.h @@ -72,15 +72,27 @@ public: void FileChanged(clang::SourceLocation sourceLocation, clang::PPCallbacks::FileChangeReason reason, clang::SrcMgr::CharacteristicKind, - clang::FileID) override + clang::FileID previousFileId) override { - if (reason == clang::PPCallbacks::EnterFile) - { - const clang::FileEntry *fileEntry = m_sourceManager->getFileEntryForID( - m_sourceManager->getFileID(sourceLocation)); - if (fileEntry) { - addFileStatus(fileEntry); - addSourceFile(fileEntry); + if (reason == clang::PPCallbacks::EnterFile) { + clang::FileID currentFileId = m_sourceManager->getFileID(sourceLocation); + if (m_mainFileId.isInvalid()) { + m_mainFileId = currentFileId; + } else { + const clang::FileEntry *fileEntry = m_sourceManager->getFileEntryForID( + currentFileId); + if (fileEntry) { + if (previousFileId == m_mainFileId) { + uint sourceFileUID = fileEntry->getUID(); + auto notAlreadyIncluded = isNotAlreadyIncluded(sourceFileUID); + if (notAlreadyIncluded.first) + m_alreadyIncludedFileUIDs.insert(notAlreadyIncluded.second, + sourceFileUID); + } else { + addFileStatus(fileEntry); + addSourceFile(fileEntry); + } + } } } } @@ -96,7 +108,8 @@ public: const clang::Module * /*imported*/, clang::SrcMgr::CharacteristicKind fileType) override { - if (file) { + clang::FileID currentFileId = m_sourceManager->getFileID(hashLocation); + if (file && currentFileId != m_mainFileId) { addSourceDependency(file, hashLocation); auto fileUID = file->getUID(); auto sourceFileUID = m_sourceManager @@ -343,6 +356,7 @@ private: BuildDependency &m_buildDependency; const std::vector &m_excludedIncludeUID; std::vector &m_alreadyIncludedFileUIDs; + clang::FileID m_mainFileId; }; } // namespace ClangBackEnd diff --git a/tests/unit/unittest/builddependencycollector-test.cpp b/tests/unit/unittest/builddependencycollector-test.cpp index 3c9ea84d32b..04bb4720adc 100644 --- a/tests/unit/unittest/builddependencycollector-test.cpp +++ b/tests/unit/unittest/builddependencycollector-test.cpp @@ -25,6 +25,8 @@ #include "googletest.h" +#include "testenvironment.h" + #include #include #include @@ -67,22 +69,15 @@ protected: { setFilePathCache(&filePathCache); - collector.addFile(id(TESTDATA_DIR "/builddependencycollector/project/main.cpp"), - {"cc", - "-I", - TESTDATA_DIR "/builddependencycollector/external", - "-I", - TESTDATA_DIR "/builddependencycollector/project", - "-isystem", - TESTDATA_DIR "/builddependencycollector/system"}); - collector.addFile(id(TESTDATA_DIR "/builddependencycollector/project/main2.cpp"), - {"cc", - "-I", - TESTDATA_DIR "/builddependencycollector/external", - "-I", - TESTDATA_DIR "/builddependencycollector/project", - "-isystem", - TESTDATA_DIR "/builddependencycollector/system"}); + collector.addFiles({id(TESTDATA_DIR "/builddependencycollector/project/main.cpp"), + id(TESTDATA_DIR "/builddependencycollector/project/main2.cpp")}, + {"cc", + "-I", + TESTDATA_DIR "/builddependencycollector/external", + "-I", + TESTDATA_DIR "/builddependencycollector/project", + "-isystem", + TESTDATA_DIR "/builddependencycollector/system"}); collector.addUnsavedFiles( {{{TESTDATA_DIR, "BuildDependencyCollector/project/generated_file.h"}, @@ -160,16 +155,18 @@ protected: protected: Sqlite::Database database{":memory:", Sqlite::JournalMode::Memory}; + TestEnvironment environment; ClangBackEnd::RefactoringDatabaseInitializer databaseInitializer{database}; ClangBackEnd::FilePathCaching filePathCache{database}; ClangBackEnd::GeneratedFiles generatedFiles; - ClangBackEnd::BuildDependencyCollector collector{filePathCache, generatedFiles}; - ClangBackEnd::BuildDependencyCollector emptyCollector{filePathCache, generatedFiles}; - ClangBackEnd::FilePaths excludePaths = {TESTDATA_DIR "/builddependencycollector/project/main.cpp", - TESTDATA_DIR "/builddependencycollector/project/main2.cpp", - TESTDATA_DIR "/builddependencycollector/project/header1.h", - TESTDATA_DIR "/builddependencycollector/project/header2.h", - TESTDATA_DIR "/builddependencycollector/project/generated_file.h"}; + ClangBackEnd::BuildDependencyCollector collector{filePathCache, generatedFiles, environment}; + ClangBackEnd::BuildDependencyCollector emptyCollector{filePathCache, generatedFiles, environment}; + ClangBackEnd::FilePaths excludePaths = { + TESTDATA_DIR "/builddependencycollector/project/main.cpp", + TESTDATA_DIR "/builddependencycollector/project/main2.cpp", + TESTDATA_DIR "/builddependencycollector/project/header1.h", + TESTDATA_DIR "/builddependencycollector/project/header2.h", + TESTDATA_DIR "/builddependencycollector/project/generated_file.h"}; }; TEST_F(BuildDependencyCollector, IncludesExternalHeader) @@ -548,10 +545,26 @@ TEST_F(BuildDependencyCollector, GeneratedFile) SourceType::UserInclude))); } +TEST_F(BuildDependencyCollector, CreateFakeFileContent) +{ + auto content = collector.generateFakeFileContent( + {id(TESTDATA_DIR "/builddependencycollector/project/header2.h"), + id(TESTDATA_DIR "/builddependencycollector/external/external1.h"), + id(TESTDATA_DIR "/builddependencycollector/external/external2.h")}); + + ASSERT_THAT(std::string(content), + AllOf(HasSubstr("#include \"" TESTDATA_DIR + "/builddependencycollector/project/header2.h\"\n"), + HasSubstr("#include \"" TESTDATA_DIR + "/builddependencycollector/external/external1.h\"\n"), + HasSubstr("#include \"" TESTDATA_DIR + "/builddependencycollector/external/external2.h\"\n"))); +} + TEST_F(BuildDependencyCollector, Create) { using ClangBackEnd::IncludeSearchPathType; - ClangBackEnd::BuildDependencyCollector collector{filePathCache, generatedFiles}; + ClangBackEnd::BuildDependencyCollector collector{filePathCache, generatedFiles, environment}; generatedFiles.update( {{TESTDATA_DIR "/builddependencycollector/project/generated_file.h", "#pragma once"}}); ClangBackEnd::ProjectPartContainer projectPart{ @@ -702,7 +715,7 @@ TEST_F(BuildDependencyCollector, Create) TEST_F(BuildDependencyCollector, Clear) { using ClangBackEnd::IncludeSearchPathType; - ClangBackEnd::BuildDependencyCollector collector{filePathCache, generatedFiles}; + ClangBackEnd::BuildDependencyCollector collector{filePathCache, generatedFiles, environment}; ClangBackEnd::ProjectPartContainer projectPart{ "project1", {},