From eadd37572adf9d0e0327bbe584f23dfd8903f1c0 Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Mon, 15 Jul 2019 11:24:58 +0200 Subject: [PATCH] Clang: Move transactions to storage We call the function anyway only isolated from other queries, so we can move the transaction guards to the project storage. Change-Id: I7cca26b25c2258856c68821671085c0a68044693 Reviewed-by: Tim Jenssen --- src/libs/clangsupport/projectpartsstorage.h | 28 +++++++++++-- .../source/symbolindexer.cpp | 2 - .../unittest/projectpartsstorage-test.cpp | 39 +++++++++++++++++++ 3 files changed, 63 insertions(+), 6 deletions(-) diff --git a/src/libs/clangsupport/projectpartsstorage.h b/src/libs/clangsupport/projectpartsstorage.h index e71bb0d51e3..e729113b49d 100644 --- a/src/libs/clangsupport/projectpartsstorage.h +++ b/src/libs/clangsupport/projectpartsstorage.h @@ -233,16 +233,36 @@ public: Utils::optional fetchProjectPartArtefact(FilePathId sourceId) const override { - ReadStatement &statement = getProjectPartArtefactsBySourceId; + try { + Sqlite::DeferredTransaction transaction{database}; - return statement.template value(sourceId.filePathId); + ReadStatement &statement = getProjectPartArtefactsBySourceId; + + auto value = statement.template value(sourceId.filePathId); + + transaction.commit(); + + return value; + } catch (const Sqlite::StatementIsBusy &) { + return fetchProjectPartArtefact(sourceId); + } } Utils::optional fetchProjectPartArtefact(ProjectPartId projectPartId) const override { - ReadStatement &statement = getProjectPartArtefactsByProjectPartId; + try { + Sqlite::DeferredTransaction transaction{database}; - return statement.template value(projectPartId.projectPathId); + ReadStatement &statement = getProjectPartArtefactsByProjectPartId; + + auto value = statement.template value(projectPartId.projectPathId); + + transaction.commit(); + + return value; + } catch (const Sqlite::StatementIsBusy &) { + return fetchProjectPartArtefact(projectPartId); + } } void resetIndexingTimeStamps(const ProjectPartContainers &projectsParts) override diff --git a/src/tools/clangrefactoringbackend/source/symbolindexer.cpp b/src/tools/clangrefactoringbackend/source/symbolindexer.cpp index c6909a07321..e3af9254645 100644 --- a/src/tools/clangrefactoringbackend/source/symbolindexer.cpp +++ b/src/tools/clangrefactoringbackend/source/symbolindexer.cpp @@ -183,12 +183,10 @@ void SymbolIndexer::updateChangedPath(FilePathId filePathId, { m_fileStatusCache.update(filePathId); - Sqlite::DeferredTransaction transaction{m_transactionInterface}; const Utils::optional optionalArtefact = m_projectPartsStorage.fetchProjectPartArtefact(filePathId); if (!optionalArtefact) return; - transaction.commit(); ProjectPartId projectPartId = optionalArtefact->projectPartId; diff --git a/tests/unit/unittest/projectpartsstorage-test.cpp b/tests/unit/unittest/projectpartsstorage-test.cpp index ba1b6f70630..892e0de3682 100644 --- a/tests/unit/unittest/projectpartsstorage-test.cpp +++ b/tests/unit/unittest/projectpartsstorage-test.cpp @@ -392,12 +392,31 @@ TEST_F(ProjectPartsStorage, UpdateProjectPartsIsBusy) TEST_F(ProjectPartsStorage, FetchProjectPartArtefactBySourceIdCallsValueInStatement) { + InSequence s; + + EXPECT_CALL(mockDatabase, deferredBegin()); EXPECT_CALL(getProjectPartArtefactsBySourceId, valueReturnProjectPartArtefact(1)) .WillRepeatedly(Return(artefact)); + EXPECT_CALL(mockDatabase, commit()); storage.fetchProjectPartArtefact(FilePathId{1}); } +TEST_F(ProjectPartsStorage, FetchProjectPartArtefactBySourceIdCallsValueInStatementIsBusy) +{ + InSequence s; + + EXPECT_CALL(mockDatabase, deferredBegin()); + EXPECT_CALL(getProjectPartArtefactsBySourceId, valueReturnProjectPartArtefact(1)) + .WillOnce(Throw(Sqlite::StatementIsBusy{""})); + EXPECT_CALL(mockDatabase, rollback()); + EXPECT_CALL(mockDatabase, deferredBegin()); + EXPECT_CALL(getProjectPartArtefactsBySourceId, valueReturnProjectPartArtefact(1)) + .WillRepeatedly(Return(artefact)); + EXPECT_CALL(mockDatabase, commit()); + + storage.fetchProjectPartArtefact(FilePathId{1}); +} TEST_F(ProjectPartsStorage, FetchProjectPartArtefactBySourceIdReturnArtefact) { EXPECT_CALL(getProjectPartArtefactsBySourceId, valueReturnProjectPartArtefact(1)) @@ -410,8 +429,12 @@ TEST_F(ProjectPartsStorage, FetchProjectPartArtefactBySourceIdReturnArtefact) TEST_F(ProjectPartsStorage, FetchProjectPartArtefactByProjectPartIdCallsValueInStatement) { + InSequence s; + + EXPECT_CALL(mockDatabase, deferredBegin()); EXPECT_CALL(getProjectPartArtefactsByProjectPartId, valueReturnProjectPartArtefact(74)) .WillRepeatedly(Return(artefact)); + EXPECT_CALL(mockDatabase, commit()); storage.fetchProjectPartArtefact(ProjectPartId{74}); } @@ -426,6 +449,22 @@ TEST_F(ProjectPartsStorage, FetchProjectPartArtefactByProjectPartIdReturnArtefac ASSERT_THAT(result, Eq(artefact)); } +TEST_F(ProjectPartsStorage, FetchProjectPartArtefactByProjectPartIdReturnArtefactIsBusy) +{ + InSequence s; + + EXPECT_CALL(mockDatabase, deferredBegin()); + EXPECT_CALL(getProjectPartArtefactsByProjectPartId, valueReturnProjectPartArtefact(74)) + .WillOnce(Throw(Sqlite::StatementIsBusy{""})); + EXPECT_CALL(mockDatabase, rollback()); + EXPECT_CALL(mockDatabase, deferredBegin()); + EXPECT_CALL(getProjectPartArtefactsByProjectPartId, valueReturnProjectPartArtefact(74)) + .WillRepeatedly(Return(artefact)); + EXPECT_CALL(mockDatabase, commit()); + + storage.fetchProjectPartArtefact(ProjectPartId{74}); +} + TEST_F(ProjectPartsStorage, ResetDependentIndexingTimeStamps) { InSequence s;