From cdda56494605035ecca52747c221c32ff75c8b23 Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Wed, 24 Jul 2019 16:13:10 +0200 Subject: [PATCH] Clang: Set indexing time stamp by dependent sources Instead of using the time stamp from clang we simply set one time stamp for all dependent sources. Change-Id: I0adbe59d46c88ddd1ac491a7f7db568bcf2ac540 Reviewed-by: Tim Jenssen --- .../source/builddependenciesstorage.h | 9 ++-- .../builddependenciesstorageinterface.h | 4 +- .../source/symbolindexer.cpp | 54 +++++++++++++++---- .../builddependenciesstorage-test.cpp | 44 +++++++++------ .../unittest/mockbuilddependenciesstorage.h | 3 ++ .../unit/unittest/mocksqlitewritestatement.h | 2 + tests/unit/unittest/pchmanagerserver-test.cpp | 2 +- tests/unit/unittest/symbolindexer-test.cpp | 6 ++- 8 files changed, 91 insertions(+), 33 deletions(-) diff --git a/src/tools/clangpchmanagerbackend/source/builddependenciesstorage.h b/src/tools/clangpchmanagerbackend/source/builddependenciesstorage.h index da078084401..aac518af5ad 100644 --- a/src/tools/clangpchmanagerbackend/source/builddependenciesstorage.h +++ b/src/tools/clangpchmanagerbackend/source/builddependenciesstorage.h @@ -177,11 +177,12 @@ public: } } - void insertOrUpdateIndexingTimeStamps(const FileStatuses &fileStatuses) override + void insertOrUpdateIndexingTimeStampsWithoutTransaction(const FilePathIds &filePathIds, + TimeStamp indexingTimeStamp) override { - for (FileStatus fileStatus : fileStatuses) { - inserOrUpdateIndexingTimesStampStatement.write(fileStatus.filePathId.filePathId, - fileStatus.lastModified); + for (FilePathId filePathId : filePathIds) { + inserOrUpdateIndexingTimesStampStatement.write(filePathId.filePathId, + indexingTimeStamp.value); } } diff --git a/src/tools/clangpchmanagerbackend/source/builddependenciesstorageinterface.h b/src/tools/clangpchmanagerbackend/source/builddependenciesstorageinterface.h index 953f2cca422..c4d102246e1 100644 --- a/src/tools/clangpchmanagerbackend/source/builddependenciesstorageinterface.h +++ b/src/tools/clangpchmanagerbackend/source/builddependenciesstorageinterface.h @@ -59,7 +59,9 @@ public: virtual FilePathIds fetchPchSources(ProjectPartId projectPartId) const = 0; virtual FilePathIds fetchSources(ProjectPartId projectPartId) const = 0; virtual void insertOrUpdateIndexingTimeStamps(const FilePathIds &filePathIds, TimeStamp indexingTimeStamp) = 0; - virtual void insertOrUpdateIndexingTimeStamps(const FileStatuses &fileStatuses) = 0; + virtual void insertOrUpdateIndexingTimeStampsWithoutTransaction(const FilePathIds &filePathIds, + TimeStamp indexingTimeStamp) + = 0; virtual SourceTimeStamps fetchIndexingTimeStamps() const = 0; virtual SourceTimeStamps fetchIncludedIndexingTimeStamps(FilePathId sourcePathId) const = 0; virtual FilePathIds fetchDependentSourceIds(const FilePathIds &sourcePathIds) const = 0; diff --git a/src/tools/clangrefactoringbackend/source/symbolindexer.cpp b/src/tools/clangrefactoringbackend/source/symbolindexer.cpp index e3af9254645..fb53a9d3a00 100644 --- a/src/tools/clangrefactoringbackend/source/symbolindexer.cpp +++ b/src/tools/clangrefactoringbackend/source/symbolindexer.cpp @@ -92,21 +92,40 @@ void SymbolIndexer::updateProjectParts(ProjectPartContainers &&projectParts) } namespace { + void store(SymbolStorageInterface &symbolStorage, BuildDependenciesStorageInterface &buildDependencyStorage, Sqlite::TransactionInterface &transactionInterface, - SymbolsCollectorInterface &symbolsCollector) + SymbolsCollectorInterface &symbolsCollector, + const FilePathIds &dependentSources) { try { + long long now = std::chrono::duration_cast( + std::chrono::system_clock::now().time_since_epoch()) + .count(); + Sqlite::ImmediateTransaction transaction{transactionInterface}; - buildDependencyStorage.insertOrUpdateIndexingTimeStamps(symbolsCollector.fileStatuses()); + buildDependencyStorage.insertOrUpdateIndexingTimeStampsWithoutTransaction(dependentSources, + now); + symbolStorage.addSymbolsAndSourceLocations(symbolsCollector.symbols(), symbolsCollector.sourceLocations()); transaction.commit(); } catch (const Sqlite::StatementIsBusy &) { - store(symbolStorage, buildDependencyStorage, transactionInterface, symbolsCollector); + store(symbolStorage, + buildDependencyStorage, + transactionInterface, + symbolsCollector, + dependentSources); } } + +FilePathIds toFilePathIds(const SourceTimeStamps &sourceTimeStamps) +{ + return Utils::transform(sourceTimeStamps, [](SourceTimeStamp sourceTimeStamp) { + return sourceTimeStamp.sourceId; + }); +} } // namespace void SymbolIndexer::updateProjectPart(ProjectPartContainer &&projectPart) @@ -124,6 +143,7 @@ void SymbolIndexer::updateProjectPart(ProjectPartContainer &&projectPart) auto indexing = [projectPart, sourcePathId, preIncludeSearchPath = m_environment.preIncludeSearchPath(), + dependentSources = toFilePathIds(dependentTimeStamps), this](SymbolsCollectorInterface &symbolsCollector) { auto collect = [&](const FilePath &pchPath) { using Builder = CommandLineBuilder; @@ -147,17 +167,20 @@ void SymbolIndexer::updateProjectPart(ProjectPartContainer &&projectPart) store(m_symbolStorage, m_buildDependencyStorage, m_transactionInterface, - symbolsCollector); + symbolsCollector, + dependentSources); } else if (pchPaths.systemPchPath.size() && collect(pchPaths.systemPchPath)) { store(m_symbolStorage, m_buildDependencyStorage, m_transactionInterface, - symbolsCollector); + symbolsCollector, + dependentSources); } else if (collect({})) { store(m_symbolStorage, m_buildDependencyStorage, m_transactionInterface, - symbolsCollector); + symbolsCollector, + dependentSources); } }; @@ -196,6 +219,7 @@ void SymbolIndexer::updateChangedPath(FilePathId filePathId, auto indexing = [optionalArtefact = std::move(optionalArtefact), filePathId, preIncludeSearchPath = m_environment.preIncludeSearchPath(), + dependentSources = toFilePathIds(dependentTimeStamps), this](SymbolsCollectorInterface &symbolsCollector) { auto collect = [&](const FilePath &pchPath) { const ProjectPartArtefact &artefact = *optionalArtefact; @@ -218,11 +242,23 @@ void SymbolIndexer::updateChangedPath(FilePathId filePathId, optionalArtefact->projectPartId); if (pchPaths.projectPchPath.size() && collect(pchPaths.projectPchPath)) { - store(m_symbolStorage, m_buildDependencyStorage, m_transactionInterface, symbolsCollector); + store(m_symbolStorage, + m_buildDependencyStorage, + m_transactionInterface, + symbolsCollector, + dependentSources); } else if (pchPaths.systemPchPath.size() && collect(pchPaths.systemPchPath)) { - store(m_symbolStorage, m_buildDependencyStorage, m_transactionInterface, symbolsCollector); + store(m_symbolStorage, + m_buildDependencyStorage, + m_transactionInterface, + symbolsCollector, + dependentSources); } else if (collect({})) { - store(m_symbolStorage, m_buildDependencyStorage, m_transactionInterface, symbolsCollector); + store(m_symbolStorage, + m_buildDependencyStorage, + m_transactionInterface, + symbolsCollector, + dependentSources); } }; diff --git a/tests/unit/unittest/builddependenciesstorage-test.cpp b/tests/unit/unittest/builddependenciesstorage-test.cpp index b542fd30821..125ac526d5e 100644 --- a/tests/unit/unittest/builddependenciesstorage-test.cpp +++ b/tests/unit/unittest/builddependenciesstorage-test.cpp @@ -302,14 +302,32 @@ TEST_F(BuildDependenciesStorage, FetchIndexingTimeStampsIsBusy) storage.fetchIndexingTimeStamps(); } +TEST_F(BuildDependenciesStorage, InsertIndexingTimeStampWithoutTransaction) +{ + InSequence s; + + EXPECT_CALL(mockDatabase, immediateBegin()).Times(0); + EXPECT_CALL(inserOrUpdateIndexingTimesStampStatement, + write(TypedEq(1), TypedEq(34))); + EXPECT_CALL(inserOrUpdateIndexingTimesStampStatement, + write(TypedEq(2), TypedEq(34))); + EXPECT_CALL(mockDatabase, commit()).Times(0); + + storage.insertOrUpdateIndexingTimeStampsWithoutTransaction({1, 2}, 34); +} + TEST_F(BuildDependenciesStorage, InsertIndexingTimeStamp) { - ClangBackEnd::FileStatuses fileStatuses{{1, 0, 34}, {2, 0, 37}}; + InSequence s; - EXPECT_CALL(inserOrUpdateIndexingTimesStampStatement, write(TypedEq(1), TypedEq(34))); - EXPECT_CALL(inserOrUpdateIndexingTimesStampStatement, write(TypedEq(2), TypedEq(37))); + EXPECT_CALL(mockDatabase, immediateBegin()); + EXPECT_CALL(inserOrUpdateIndexingTimesStampStatement, + write(TypedEq(1), TypedEq(34))); + EXPECT_CALL(inserOrUpdateIndexingTimesStampStatement, + write(TypedEq(2), TypedEq(34))); + EXPECT_CALL(mockDatabase, commit()); - storage.insertOrUpdateIndexingTimeStamps(fileStatuses); + storage.insertOrUpdateIndexingTimeStamps({1, 2}, 34); } TEST_F(BuildDependenciesStorage, InsertIndexingTimeStampsIsBusy) @@ -318,8 +336,10 @@ TEST_F(BuildDependenciesStorage, InsertIndexingTimeStampsIsBusy) EXPECT_CALL(mockDatabase, immediateBegin()).WillOnce(Throw(Sqlite::StatementIsBusy{""})); EXPECT_CALL(mockDatabase, immediateBegin()); - EXPECT_CALL(inserOrUpdateIndexingTimesStampStatement, write(TypedEq(1), TypedEq(34))); - EXPECT_CALL(inserOrUpdateIndexingTimesStampStatement, write(TypedEq(2), TypedEq(34))); + EXPECT_CALL(inserOrUpdateIndexingTimesStampStatement, + write(TypedEq(1), TypedEq(34))); + EXPECT_CALL(inserOrUpdateIndexingTimesStampStatement, + write(TypedEq(2), TypedEq(34))); EXPECT_CALL(mockDatabase, commit()); storage.insertOrUpdateIndexingTimeStamps({1, 2}, 34); @@ -386,19 +406,11 @@ TEST_F(BuildDependenciesStorageSlow, UpdateIndexingTimeStamps) ElementsAre(SourceTimeStamp{1, 37}, SourceTimeStamp{2, 34})); } -TEST_F(BuildDependenciesStorageSlow, InsertIndexingTimeStamp) -{ - storage.insertOrUpdateIndexingTimeStamps({{1, 0, 34}, {2, 0, 37}}); - - ASSERT_THAT(storage.fetchIndexingTimeStamps(), - ElementsAre(SourceTimeStamp{1, 34}, SourceTimeStamp{2, 37})); -} - TEST_F(BuildDependenciesStorageSlow, UpdateIndexingTimeStamp) { - storage.insertOrUpdateIndexingTimeStamps({{1, 0, 34}, {2, 0, 34}}); + storage.insertOrUpdateIndexingTimeStamps({1, 2}, 34); - storage.insertOrUpdateIndexingTimeStamps({{2, 0, 37}}); + storage.insertOrUpdateIndexingTimeStamps({2}, 37); ASSERT_THAT(storage.fetchIndexingTimeStamps(), ElementsAre(SourceTimeStamp{1, 34}, SourceTimeStamp{2, 37})); diff --git a/tests/unit/unittest/mockbuilddependenciesstorage.h b/tests/unit/unittest/mockbuilddependenciesstorage.h index 0436e27f92b..c98b3fd3aeb 100644 --- a/tests/unit/unittest/mockbuilddependenciesstorage.h +++ b/tests/unit/unittest/mockbuilddependenciesstorage.h @@ -58,6 +58,9 @@ public: MOCK_METHOD2(insertOrUpdateIndexingTimeStamps, void(const ClangBackEnd::FilePathIds &filePathIds, ClangBackEnd::TimeStamp indexingTimeStamp)); + MOCK_METHOD2(insertOrUpdateIndexingTimeStampsWithoutTransaction, + void(const ClangBackEnd::FilePathIds &filePathIds, + ClangBackEnd::TimeStamp indexingTimeStamp)); MOCK_METHOD1(insertOrUpdateIndexingTimeStamps, void(const ClangBackEnd::FileStatuses &)); MOCK_CONST_METHOD0(fetchIndexingTimeStamps, ClangBackEnd::SourceTimeStamps()); MOCK_CONST_METHOD1(fetchIncludedIndexingTimeStamps, diff --git a/tests/unit/unittest/mocksqlitewritestatement.h b/tests/unit/unittest/mocksqlitewritestatement.h index 5644fd9bb87..329c5f7f39a 100644 --- a/tests/unit/unittest/mocksqlitewritestatement.h +++ b/tests/unit/unittest/mocksqlitewritestatement.h @@ -95,6 +95,8 @@ public: MOCK_METHOD1(write, void (int)); + MOCK_METHOD2(write, void(int, long long)); + MOCK_METHOD2(write, void (int, int)); diff --git a/tests/unit/unittest/pchmanagerserver-test.cpp b/tests/unit/unittest/pchmanagerserver-test.cpp index 4657051fc59..94b7d82e286 100644 --- a/tests/unit/unittest/pchmanagerserver-test.cpp +++ b/tests/unit/unittest/pchmanagerserver-test.cpp @@ -290,7 +290,7 @@ TEST_F(PchManagerServer, ResetTimeStampForChangedFilesInDatabase) { EXPECT_CALL(mockBuildDependenciesStorage, insertOrUpdateIndexingTimeStamps(ElementsAre(FilePathId{1}, FilePathId{3}, FilePathId{5}), - TypedEq(0))); + TypedEq(-1))); server.pathsChanged({1, 3, 5}); } diff --git a/tests/unit/unittest/symbolindexer-test.cpp b/tests/unit/unittest/symbolindexer-test.cpp index 86d0072c904..87d5ba3710b 100644 --- a/tests/unit/unittest/symbolindexer-test.cpp +++ b/tests/unit/unittest/symbolindexer-test.cpp @@ -872,7 +872,8 @@ TEST_F(SymbolIndexer, UpdateProjectPartsFetchIncludedIndexingTimeStamps) EXPECT_CALL(mockSqliteTransactionBackend, immediateBegin()); EXPECT_CALL(mockCollector, fileStatuses()).WillRepeatedly(ReturnRef(fileStatuses1)); - EXPECT_CALL(mockBuildDependenciesStorage, insertOrUpdateIndexingTimeStamps(_)); + EXPECT_CALL(mockBuildDependenciesStorage, + insertOrUpdateIndexingTimeStampsWithoutTransaction(_, _)); EXPECT_CALL(mockSymbolStorage, addSymbolsAndSourceLocations(_, _)); EXPECT_CALL(mockSqliteTransactionBackend, commit()); @@ -887,7 +888,8 @@ TEST_F(SymbolIndexer, UpdateProjectPartsIsBusyInStoringData) .WillOnce(Throw(Sqlite::StatementIsBusy{""})); EXPECT_CALL(mockSqliteTransactionBackend, immediateBegin()); EXPECT_CALL(mockCollector, fileStatuses()).WillRepeatedly(ReturnRef(fileStatuses1)); - EXPECT_CALL(mockBuildDependenciesStorage, insertOrUpdateIndexingTimeStamps(_)); + EXPECT_CALL(mockBuildDependenciesStorage, + insertOrUpdateIndexingTimeStampsWithoutTransaction(_, _)); EXPECT_CALL(mockSymbolStorage, addSymbolsAndSourceLocations(_, _)); EXPECT_CALL(mockSqliteTransactionBackend, commit());