From cc78969bdb071e0339206d3e42f670223d00d35b Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Wed, 24 Jul 2019 16:23:08 +0200 Subject: [PATCH] Clang: Improve updating of PCHs Instead of deleting the path and time stamp of the project PCH we always set the time stamp but delete only the path if PCH creation failed. A failed PCH will be no rebuilt if the files changed. We have to add that feature later. Change-Id: I1094271f9ead5d906e94b68ac91c0becd2371ca9 Reviewed-by: Tim Jenssen --- src/libs/clangsupport/projectpartcontainer.h | 19 ++++----- src/libs/clangsupport/projectpartsstorage.h | 18 ++++----- .../source/pchtaskqueue.cpp | 3 +- .../source/precompiledheaderstorage.h | 11 ++++-- .../precompiledheaderstorageinterface.h | 2 +- .../unittest/mockprecompiledheaderstorage.h | 3 +- tests/unit/unittest/pchtaskqueue-test.cpp | 4 +- .../precompiledheaderstorage-test.cpp | 12 ++++-- .../unittest/projectpartsmanager-test.cpp | 2 +- .../unittest/projectpartsstorage-test.cpp | 39 ++++++++++--------- 10 files changed, 61 insertions(+), 52 deletions(-) diff --git a/src/libs/clangsupport/projectpartcontainer.h b/src/libs/clangsupport/projectpartcontainer.h index b2133dcaa29..caededee5c8 100644 --- a/src/libs/clangsupport/projectpartcontainer.h +++ b/src/libs/clangsupport/projectpartcontainer.h @@ -32,6 +32,7 @@ #include "includesearchpath.h" #include "projectpartartefact.h" #include "projectpartid.h" +#include "sourceentry.h" #include #include @@ -126,12 +127,12 @@ public: friend bool operator==(const ProjectPartContainer &first, const ProjectPartContainer &second) { return first.projectPartId == second.projectPartId - && first.toolChainArguments == second.toolChainArguments - && first.compilerMacros == second.compilerMacros - && first.systemIncludeSearchPaths == second.systemIncludeSearchPaths - && first.projectIncludeSearchPaths == second.projectIncludeSearchPaths - && first.headerPathIds == second.headerPathIds - && first.sourcePathIds == second.sourcePathIds&& first.language == second.language + && first.toolChainArguments == second.toolChainArguments + && first.compilerMacros == second.compilerMacros + && first.systemIncludeSearchPaths == second.systemIncludeSearchPaths + && first.projectIncludeSearchPaths == second.projectIncludeSearchPaths + && first.headerPathIds == second.headerPathIds + && first.sourcePathIds == second.sourcePathIds && first.language == second.language && first.languageVersion == second.languageVersion && first.languageExtension == second.languageExtension; } @@ -148,7 +149,7 @@ public: first.language, first.languageVersion, first.languageExtension, - first.hasPrecompiledHeader) + first.preCompiledHeaderWasGenerated) < std::tie(second.projectPartId, second.toolChainArguments, second.compilerMacros, @@ -159,7 +160,7 @@ public: second.language, second.languageVersion, second.languageExtension, - second.hasPrecompiledHeader); + second.preCompiledHeaderWasGenerated); } ProjectPartContainer clone() const @@ -171,7 +172,7 @@ public: FilePathIds headerPathIds; FilePathIds sourcePathIds; bool updateIsDeferred = false; - bool hasPrecompiledHeader = true; + bool preCompiledHeaderWasGenerated = true; }; using ProjectPartContainerReference = std::reference_wrapper; diff --git a/src/libs/clangsupport/projectpartsstorage.h b/src/libs/clangsupport/projectpartsstorage.h index d3eb01064d3..4ff1789c93b 100644 --- a/src/libs/clangsupport/projectpartsstorage.h +++ b/src/libs/clangsupport/projectpartsstorage.h @@ -74,12 +74,12 @@ public: .template values(1024, projectPartId.projectPathId); } - bool hasPrecompiledHeader(ProjectPartId projectPartId) const + bool preCompiledHeaderWasGenerated(ProjectPartId projectPartId) const { - auto value = fetchProjectPrecompiledHeaderPathStatement.template value( + auto value = fetchProjectPrecompiledHeaderBuildTimeStatement.template value( projectPartId.projectPathId); - return value && value->hasContent(); + return value && *value > 0; } ProjectPartContainers fetchProjectParts(const ProjectPartIds &projectPartIds) const override @@ -96,7 +96,7 @@ public: if (value) { value->headerPathIds = fetchHeaders(projectPartId); value->sourcePathIds = fetchSources(projectPartId); - value->hasPrecompiledHeader = hasPrecompiledHeader(projectPartId); + value->preCompiledHeaderWasGenerated = preCompiledHeaderWasGenerated(projectPartId); projectParts.push_back(*std::move(value)); } } @@ -379,13 +379,9 @@ public: mutable ReadStatement fetchProjectPartsSourcesByIdStatement{ "SELECT sourceId FROM projectPartsSources WHERE projectPartId = ? ORDER BY sourceId", database}; - mutable ReadStatement fetchProjectPrecompiledHeaderPathStatement{ - "SELECT projectPchPath FROM precompiledHeaders WHERE projectPartId = ?", database}; + mutable ReadStatement fetchProjectPrecompiledHeaderBuildTimeStatement{ + "SELECT projectPchBuildTime FROM precompiledHeaders WHERE projectPartId = ?", database}; WriteStatement resetDependentIndexingTimeStampsStatement{ - "WITH RECURSIVE collectedDependencies(sourceId) AS (VALUES(?) UNION SELECT " - "dependencySourceId FROM sourceDependencies, collectedDependencies WHERE " - "sourceDependencies.sourceId == collectedDependencies.sourceId) UPDATE fileStatuses SET " - "indexingTimeStamp = NULL WHERE sourceId IN (SELECT sourceId FROM collectedDependencies)", - database}; + "UPDATE fileStatuses SET indexingTimeStamp = NULL WHERE sourceId = ?", database}; }; } // namespace ClangBackEnd diff --git a/src/tools/clangpchmanagerbackend/source/pchtaskqueue.cpp b/src/tools/clangpchmanagerbackend/source/pchtaskqueue.cpp index b9d6a48fe39..7a6e29c584c 100644 --- a/src/tools/clangpchmanagerbackend/source/pchtaskqueue.cpp +++ b/src/tools/clangpchmanagerbackend/source/pchtaskqueue.cpp @@ -153,7 +153,8 @@ std::vector PchTaskQueue::createProjectTasks(PchTasks &&pchT pchCreator.generatePch(std::move(pchTask)); const auto &projectPartPch = pchCreator.projectPartPch(); if (projectPartPch.pchPath.empty()) { - m_precompiledHeaderStorage.deleteProjectPrecompiledHeader(projectPartId); + m_precompiledHeaderStorage.deleteProjectPrecompiledHeader(projectPartId, + projectPartPch.lastModified); } else { m_precompiledHeaderStorage.insertProjectPrecompiledHeader( projectPartId, projectPartPch.pchPath, projectPartPch.lastModified); diff --git a/src/tools/clangpchmanagerbackend/source/precompiledheaderstorage.h b/src/tools/clangpchmanagerbackend/source/precompiledheaderstorage.h index 5510376fd53..085e146d18e 100644 --- a/src/tools/clangpchmanagerbackend/source/precompiledheaderstorage.h +++ b/src/tools/clangpchmanagerbackend/source/precompiledheaderstorage.h @@ -65,16 +65,17 @@ public: } } - void deleteProjectPrecompiledHeader(ProjectPartId projectPartId) override + void deleteProjectPrecompiledHeader(ProjectPartId projectPartId, long long pchBuildTime) override { try { Sqlite::ImmediateTransaction transaction{database}; - deleteProjectPrecompiledHeaderStatement.write(projectPartId.projectPathId); + deleteProjectPrecompiledHeaderPathAndSetBuildTimeStatement.write(projectPartId.projectPathId, + pchBuildTime); transaction.commit(); } catch (const Sqlite::StatementIsBusy) { - deleteProjectPrecompiledHeader(projectPartId); + deleteProjectPrecompiledHeader(projectPartId, pchBuildTime); } } @@ -198,6 +199,10 @@ public: "UPDATE OR IGNORE precompiledHeaders SET projectPchPath=NULL,projectPchBuildTime=NULL " "WHERE projectPartId = ?", database}; + WriteStatement deleteProjectPrecompiledHeaderPathAndSetBuildTimeStatement{ + "UPDATE OR IGNORE precompiledHeaders SET projectPchPath=NULL,projectPchBuildTime=?002 " + "WHERE projectPartId = ?001", + database}; WriteStatement deleteSystemPrecompiledHeaderStatement{ "UPDATE OR IGNORE precompiledHeaders SET systemPchPath=NULL,systemPchBuildTime=NULL " "WHERE projectPartId = ?", diff --git a/src/tools/clangpchmanagerbackend/source/precompiledheaderstorageinterface.h b/src/tools/clangpchmanagerbackend/source/precompiledheaderstorageinterface.h index 2ce880cd6a7..f2c000072f2 100644 --- a/src/tools/clangpchmanagerbackend/source/precompiledheaderstorageinterface.h +++ b/src/tools/clangpchmanagerbackend/source/precompiledheaderstorageinterface.h @@ -47,7 +47,7 @@ public: Utils::SmallStringView pchPath, long long pchBuildTime) = 0; - virtual void deleteProjectPrecompiledHeader(ProjectPartId projectPartId) = 0; + virtual void deleteProjectPrecompiledHeader(ProjectPartId projectPartId, long long pchBuildTime) = 0; virtual void deleteProjectPrecompiledHeaders(const ProjectPartIds &projectPartIds) = 0; virtual void insertSystemPrecompiledHeaders(const ProjectPartIds &projectPartIds, Utils::SmallStringView pchPath, diff --git a/tests/unit/unittest/mockprecompiledheaderstorage.h b/tests/unit/unittest/mockprecompiledheaderstorage.h index 84caea55be5..45c2119a1e3 100644 --- a/tests/unit/unittest/mockprecompiledheaderstorage.h +++ b/tests/unit/unittest/mockprecompiledheaderstorage.h @@ -36,7 +36,8 @@ public: void(ClangBackEnd::ProjectPartId projectPartId, Utils::SmallStringView pchPath, long long pchBuildTime)); - MOCK_METHOD1(deleteProjectPrecompiledHeader, void(ClangBackEnd::ProjectPartId projectPartId)); + MOCK_METHOD2(deleteProjectPrecompiledHeader, + void(ClangBackEnd::ProjectPartId projectPartId, long long buildTime)); MOCK_METHOD1(deleteProjectPrecompiledHeaders, void(const ClangBackEnd::ProjectPartIds &projectPartIds)); MOCK_METHOD3(insertSystemPrecompiledHeaders, diff --git a/tests/unit/unittest/pchtaskqueue-test.cpp b/tests/unit/unittest/pchtaskqueue-test.cpp index 0715602f97d..38609c72770 100644 --- a/tests/unit/unittest/pchtaskqueue-test.cpp +++ b/tests/unit/unittest/pchtaskqueue-test.cpp @@ -343,7 +343,7 @@ TEST_F(PchTaskQueue, DeleteProjectPchEntryInDatabaseIfNoPchIsGenerated) { InSequence s; MockPchCreator mockPchCreator; - ClangBackEnd::ProjectPartPch projectPartPch{{}, "", 0}; + ClangBackEnd::ProjectPartPch projectPartPch{{}, "", 34}; auto tasks = queue.createProjectTasks({projectTask1}); auto projectTask = projectTask1; projectTask.systemPchPath = "/path/to/pch"; @@ -353,7 +353,7 @@ TEST_F(PchTaskQueue, DeleteProjectPchEntryInDatabaseIfNoPchIsGenerated) .WillOnce(Return(ClangBackEnd::FilePath{"/path/to/pch"})); EXPECT_CALL(mockPchCreator, generatePch(Eq(projectTask))); EXPECT_CALL(mockPchCreator, projectPartPch()).WillOnce(ReturnRef(projectPartPch)); - EXPECT_CALL(mockPrecompiledHeaderStorage, deleteProjectPrecompiledHeader(Eq(1))); + EXPECT_CALL(mockPrecompiledHeaderStorage, deleteProjectPrecompiledHeader(Eq(1), Eq(34))); tasks.front()(mockPchCreator); } diff --git a/tests/unit/unittest/precompiledheaderstorage-test.cpp b/tests/unit/unittest/precompiledheaderstorage-test.cpp index 9fb161d0eb5..d01f724df33 100644 --- a/tests/unit/unittest/precompiledheaderstorage-test.cpp +++ b/tests/unit/unittest/precompiledheaderstorage-test.cpp @@ -44,6 +44,8 @@ protected: Storage storage{database}; MockSqliteWriteStatement &insertProjectPrecompiledHeaderStatement = storage.insertProjectPrecompiledHeaderStatement; MockSqliteWriteStatement &deleteProjectPrecompiledHeaderStatement = storage.deleteProjectPrecompiledHeaderStatement; + MockSqliteWriteStatement &deleteProjectPrecompiledHeaderPathAndSetBuildTimeStatement + = storage.deleteProjectPrecompiledHeaderPathAndSetBuildTimeStatement; MockSqliteWriteStatement &insertSystemPrecompiledHeaderStatement = storage.insertSystemPrecompiledHeaderStatement; MockSqliteWriteStatement &deleteSystemPrecompiledHeaderStatement = storage.deleteSystemPrecompiledHeaderStatement; MockSqliteReadStatement &fetchSystemPrecompiledHeaderPathStatement = storage.fetchSystemPrecompiledHeaderPathStatement; @@ -95,10 +97,11 @@ TEST_F(PrecompiledHeaderStorage, DeleteProjectPrecompiledHeader) InSequence s; EXPECT_CALL(database, immediateBegin()); - EXPECT_CALL(deleteProjectPrecompiledHeaderStatement, write(TypedEq(1))); + EXPECT_CALL(deleteProjectPrecompiledHeaderPathAndSetBuildTimeStatement, + write(TypedEq(1), TypedEq(13))); EXPECT_CALL(database, commit()); - storage.deleteProjectPrecompiledHeader(1); + storage.deleteProjectPrecompiledHeader(1, 13); } TEST_F(PrecompiledHeaderStorage, DeleteProjectPrecompiledHeaderStatementIsBusy) @@ -107,10 +110,11 @@ TEST_F(PrecompiledHeaderStorage, DeleteProjectPrecompiledHeaderStatementIsBusy) EXPECT_CALL(database, immediateBegin()).WillOnce(Throw(Sqlite::StatementIsBusy("busy"))); EXPECT_CALL(database, immediateBegin()); - EXPECT_CALL(deleteProjectPrecompiledHeaderStatement, write(TypedEq(1))); + EXPECT_CALL(deleteProjectPrecompiledHeaderPathAndSetBuildTimeStatement, + write(TypedEq(1), TypedEq(13))); EXPECT_CALL(database, commit()); - storage.deleteProjectPrecompiledHeader(1); + storage.deleteProjectPrecompiledHeader(1, 13); } TEST_F(PrecompiledHeaderStorage, DeleteProjectPrecompiledHeaders) diff --git a/tests/unit/unittest/projectpartsmanager-test.cpp b/tests/unit/unittest/projectpartsmanager-test.cpp index d86543e9c26..27d1142689a 100644 --- a/tests/unit/unittest/projectpartsmanager-test.cpp +++ b/tests/unit/unittest/projectpartsmanager-test.cpp @@ -43,7 +43,7 @@ class ProjectPartsManager : public testing::Test protected: ProjectPartsManager() { - projectPartContainerWithoutPrecompiledHeader1.hasPrecompiledHeader = false; + projectPartContainerWithoutPrecompiledHeader1.preCompiledHeaderWasGenerated = false; } NiceMock mockProjectPartsStorage; NiceMock mockPrecompiledHeaderStorage; diff --git a/tests/unit/unittest/projectpartsstorage-test.cpp b/tests/unit/unittest/projectpartsstorage-test.cpp index 892e0de3682..6c887ca3f06 100644 --- a/tests/unit/unittest/projectpartsstorage-test.cpp +++ b/tests/unit/unittest/projectpartsstorage-test.cpp @@ -104,7 +104,8 @@ protected: MockSqliteWriteStatement &insertProjectPartsSourcesStatement = storage.insertProjectPartsSourcesStatement; MockSqliteReadStatement &fetchProjectPartsHeadersByIdStatement = storage.fetchProjectPartsHeadersByIdStatement; MockSqliteReadStatement &fetchProjectPartsSourcesByIdStatement = storage.fetchProjectPartsSourcesByIdStatement; - MockSqliteReadStatement &fetchProjectPrecompiledHeaderPathStatement = storage.fetchProjectPrecompiledHeaderPathStatement; + MockSqliteReadStatement &fetchProjectPrecompiledHeaderPathStatement = storage.fetchProjectPrecompiledHeaderBuildTimeStatement; + MockSqliteReadStatement &fetchProjectPrecompiledHeaderBuildTimeStatement = storage.fetchProjectPrecompiledHeaderBuildTimeStatement; MockSqliteWriteStatement &resetDependentIndexingTimeStampsStatement = storage.resetDependentIndexingTimeStampsStatement; IncludeSearchPaths systemIncludeSearchPaths{{"/includes", 1, IncludeSearchPathType::BuiltIn}, {"/other/includes", 2, IncludeSearchPathType::System}}; @@ -257,11 +258,11 @@ TEST_F(ProjectPartsStorage, FetchProjectPartsByIds) EXPECT_CALL(fetchProjectPartByIdStatement, valueReturnProjectPartContainer(Eq(1))); EXPECT_CALL(fetchProjectPartsHeadersByIdStatement, valuesReturnFilePathIds(1024, Eq(1))); EXPECT_CALL(fetchProjectPartsSourcesByIdStatement, valuesReturnFilePathIds(1024, Eq(1))); - EXPECT_CALL(fetchProjectPrecompiledHeaderPathStatement, valueReturnSmallString(Eq(1))); + EXPECT_CALL(fetchProjectPrecompiledHeaderBuildTimeStatement, valueReturnInt64(Eq(1))); EXPECT_CALL(fetchProjectPartByIdStatement, valueReturnProjectPartContainer(Eq(2))); EXPECT_CALL(fetchProjectPartsHeadersByIdStatement, valuesReturnFilePathIds(1024, Eq(2))); EXPECT_CALL(fetchProjectPartsSourcesByIdStatement, valuesReturnFilePathIds(1024, Eq(2))); - EXPECT_CALL(fetchProjectPrecompiledHeaderPathStatement, valueReturnSmallString(Eq(2))); + EXPECT_CALL(fetchProjectPrecompiledHeaderBuildTimeStatement, valueReturnInt64(Eq(2))); EXPECT_CALL(mockDatabase, commit()); storage.fetchProjectParts({1, 2}); @@ -284,34 +285,34 @@ TEST_F(ProjectPartsStorage, FetchProjectPartsByIdsIsBusy) storage.fetchProjectParts({1, 2}); } -TEST_F(ProjectPartsStorage, FetchProjectPartsByIdsHasPrecompiledNullOptional) +TEST_F(ProjectPartsStorage, FetchProjectPartsByIdsPreCompiledHeaderWasGeneratedNullOptional) { - ON_CALL(fetchProjectPrecompiledHeaderPathStatement, valueReturnSmallString(Eq(1))) - .WillByDefault(Return(Utils::optional{})); + ON_CALL(fetchProjectPrecompiledHeaderBuildTimeStatement, valueReturnInt64(Eq(1))) + .WillByDefault(Return(Utils::optional{})); auto projectParts = storage.fetchProjectParts({1}); - ASSERT_FALSE(projectParts.front().hasPrecompiledHeader); + ASSERT_FALSE(projectParts.front().preCompiledHeaderWasGenerated); } -TEST_F(ProjectPartsStorage, FetchProjectPartsByIdsHasPrecompiledEmptyString) +TEST_F(ProjectPartsStorage, FetchProjectPartsByIdsPreCompiledHeaderWasGeneratedZero) { - ON_CALL(fetchProjectPrecompiledHeaderPathStatement, valueReturnSmallString(Eq(1))) - .WillByDefault(Return(Utils::optional{""})); + ON_CALL(fetchProjectPrecompiledHeaderBuildTimeStatement, valueReturnInt64(Eq(1))) + .WillByDefault(Return(Utils::optional{0})); auto projectParts = storage.fetchProjectParts({1}); - ASSERT_FALSE(projectParts.front().hasPrecompiledHeader); + ASSERT_FALSE(projectParts.front().preCompiledHeaderWasGenerated); } -TEST_F(ProjectPartsStorage, FetchProjectPartsByIdsHasPrecompiledStringWithContent) +TEST_F(ProjectPartsStorage, FetchProjectPartsByIdsPreCompiledHeaderWasGeneratedSomeNumber) { - ON_CALL(fetchProjectPrecompiledHeaderPathStatement, valueReturnSmallString(Eq(1))) - .WillByDefault(Return(Utils::optional{"/some/path"})); + ON_CALL(fetchProjectPrecompiledHeaderBuildTimeStatement, valueReturnInt64(Eq(1))) + .WillByDefault(Return(Utils::optional{23})); auto projectParts = storage.fetchProjectParts({1}); - ASSERT_TRUE(projectParts.front().hasPrecompiledHeader); + ASSERT_TRUE(projectParts.front().preCompiledHeaderWasGenerated); } TEST_F(ProjectPartsStorage, FetchProjectPartsByIdsHasMissingId) @@ -550,12 +551,12 @@ TEST_F(ProjectPartsStorageSlow, ResetDependentIndexingTimeStamps) storage.resetIndexingTimeStamps({projectPart1, projectPart2}); ASSERT_THAT(buildDependenciesStorage.fetchIndexingTimeStamps(), - ElementsAre(SourceTimeStamp{1, 0}, - SourceTimeStamp{2, 0}, + ElementsAre(SourceTimeStamp{1, 34}, + SourceTimeStamp{2, 34}, SourceTimeStamp{3, 0}, SourceTimeStamp{4, 0}, - SourceTimeStamp{5, 0}, - SourceTimeStamp{6, 0}, + SourceTimeStamp{5, 34}, + SourceTimeStamp{6, 34}, SourceTimeStamp{7, 0}, SourceTimeStamp{8, 0}, SourceTimeStamp{9, 34},