Clang: Bulk add project file paths to database

This project part container generation because there is not anymore one
single access to the database for every file path.

Change-Id: I5f82022262fe89a976729d48ee4f098b74a1e1d1
Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>

src/libs/clangsupport/filepathcache.h

@@ -33,6 +33,8 @@
 #include "filepathview.h"
 #include "stringcache.h"
 
+#include <sqlitetransaction.h>
+
 #include <algorithm>
 
 namespace ClangBackEnd {
@@ -161,25 +163,35 @@ public:
         return m_fileNameCache.string(filePathId.filePathId, fetchSoureNameAndDirectoryId).directoryId;
     }
 
-    void addFilePaths(FilePathViews &&filePaths)
+    template<typename Container>
+    void addFilePaths(Container &&filePaths)
     {
         auto directoryPaths = Utils::transform<std::vector<Utils::SmallStringView>>(
             filePaths, [](FilePathView filePath) { return filePath.directory(); });
 
-        m_directoryPathCache.addStrings(std::move(directoryPaths),
+        std::unique_ptr<Sqlite::DeferredTransaction> transaction;
-                                        [&](Utils::SmallStringView directoryPath) {
+
-                                            return m_filePathStorage.fetchDirectoryIdUnguarded(
+        m_directoryPathCache.addStrings(std::move(directoryPaths), [&](Utils::SmallStringView directoryPath) {
-                                                directoryPath);
+            if (!transaction)
-                                        });
+                transaction = std::make_unique<Sqlite::DeferredTransaction>(
+                    m_filePathStorage.database());
+            return m_filePathStorage.fetchDirectoryIdUnguarded(directoryPath);
+        });
 
         auto sourcePaths = Utils::transform<std::vector<FileNameView>>(filePaths, [&](FilePathView filePath) {
             return FileNameView{filePath.name(), m_directoryPathCache.stringId(filePath.directory())};
         });
 
         m_fileNameCache.addStrings(std::move(sourcePaths), [&](FileNameView fileNameView) {
+            if (!transaction)
+                transaction = std::make_unique<Sqlite::DeferredTransaction>(
+                    m_filePathStorage.database());
             return m_filePathStorage.fetchSourceIdUnguarded(fileNameView.directoryId,
                                                             fileNameView.fileName);
         });
+
+        if (transaction)
+            transaction->commit();
     }
 
 private:

src/libs/clangsupport/filepathcaching.cpp

@@ -52,6 +52,11 @@ DirectoryPathId FilePathCaching::directoryPathId(FilePathId filePathId) const
     return m_cache.directoryPathId(filePathId);
 }
 
+void FilePathCaching::addFilePaths(const FilePaths &filePaths)
+{
+    m_cache.addFilePaths(filePaths);
+}
+
 FilePathId CopyableFilePathCaching::filePathId(FilePathView filePath) const
 {
     return m_cache.filePathId(filePath);
@@ -77,4 +82,9 @@ DirectoryPathId CopyableFilePathCaching::directoryPathId(FilePathId filePathId)
     return m_cache.directoryPathId(filePathId);
 }
 
+void CopyableFilePathCaching::addFilePaths(const FilePaths &filePaths)
+{
+    m_cache.addFilePaths(filePaths);
+}
+
 } // namespace ClangBackEnd

src/libs/clangsupport/filepathcaching.h

@@ -55,6 +55,7 @@ public:
     DirectoryPathId directoryPathId(Utils::SmallStringView directoryPath) const override;
     Utils::PathString directoryPath(DirectoryPathId directoryPathId) const override;
     DirectoryPathId directoryPathId(FilePathId filePathId) const override;
+    void addFilePaths(const ClangBackEnd::FilePaths &filePaths) override;
 
 private:
     Factory m_factory;
@@ -78,6 +79,7 @@ public:
     DirectoryPathId directoryPathId(Utils::SmallStringView directoryPath) const override;
     Utils::PathString directoryPath(DirectoryPathId directoryPathId) const override;
     DirectoryPathId directoryPathId(FilePathId filePathId) const override;
+    void addFilePaths(const ClangBackEnd::FilePaths &filePaths) override;
 
 private:
     Cache m_cache;

src/libs/clangsupport/filepathcachinginterface.h

@@ -44,6 +44,7 @@ public:
     virtual DirectoryPathId directoryPathId(Utils::SmallStringView directoryPath) const = 0;
     virtual DirectoryPathId directoryPathId(FilePathId filePathId) const = 0;
     virtual Utils::PathString directoryPath(DirectoryPathId directoryPathId) const = 0;
+    virtual void addFilePaths(const ClangBackEnd::FilePaths &filePaths) = 0;
 
     template<typename Container>
     FilePathIds filePathIds(Container &&filePaths) const

src/libs/clangsupport/filepathstorage.h

@@ -235,6 +235,8 @@ public:
         }
     }
 
+    Database &database() { return m_statementFactory.database; }
+
 private:
     StatementFactory &m_statementFactory;
 };

src/plugins/clangpchmanager/projectupdater.cpp

@@ -67,8 +67,11 @@ public:
 void ProjectUpdater::updateProjectParts(const std::vector<CppTools::ProjectPart *> &projectParts,
                                         Utils::SmallStringVector &&toolChainArguments)
 {
-    m_server.updateProjectParts(ClangBackEnd::UpdateProjectPartsMessage{
+    addProjectFilesToFilePathCache(projectParts);
-        toProjectPartContainers(projectParts), std::move(toolChainArguments)});
+
+    m_server.updateProjectParts(
+        ClangBackEnd::UpdateProjectPartsMessage{toProjectPartContainers(projectParts),
+                                                std::move(toolChainArguments)});
 }
 
 void ProjectUpdater::removeProjectParts(ClangBackEnd::ProjectPartIds projectPartIds)
@@ -432,4 +435,24 @@ ClangBackEnd::ProjectPartIds ProjectUpdater::toProjectPartIds(
     return projectPartIds;
 }
 
+void ProjectUpdater::addProjectFilesToFilePathCache(const std::vector<CppTools::ProjectPart *> &projectParts)
+{
+    std::size_t fileCount = std::accumulate(projectParts.begin(),
+                                            projectParts.end(),
+                                            std::size_t(0),
+                                            [](std::size_t value, CppTools::ProjectPart *projectPart) {
+                                                return value + std::size_t(projectPart->files.size());
+                                            });
+    ClangBackEnd::FilePaths filePaths;
+    filePaths.reserve(fileCount);
+
+    for (CppTools::ProjectPart *projectPart : projectParts) {
+        for (const CppTools::ProjectFile &file : projectPart->files)
+            if (file.active)
+                filePaths.emplace_back(file.path);
+    }
+
+    m_filePathCache.addFilePaths(filePaths);
+}
+
 } // namespace ClangPchManager

src/plugins/clangpchmanager/projectupdater.h

@@ -115,6 +115,9 @@ public:
 
     ClangBackEnd::ProjectPartIds toProjectPartIds(const QStringList &projectPartNames) const;
 
+private:
+    void addProjectFilesToFilePathCache(const std::vector<CppTools::ProjectPart *> &projectParts);
+
 private:
     ClangBackEnd::GeneratedFiles m_generatedFiles;
     ClangBackEnd::FilePaths m_excludedPaths;

tests/unit/unittest/filepathcache-test.cpp

@@ -26,6 +26,7 @@
 #include "googletest.h"
 
 #include "mockfilepathstorage.h"
+#include "mocksqlitedatabase.h"
 
 #include <filepathcache.h>
 
@@ -37,6 +38,7 @@ using Cache = ClangBackEnd::FilePathCache<NiceMock<MockFilePathStorage>>;
 using ClangBackEnd::FilePathId;
 using NFP = ClangBackEnd::FilePath;
 using ClangBackEnd::FilePathView;
+using ClangBackEnd::FilePathViews;
 using ClangBackEnd::Sources::SourceNameAndDirectoryId;
 
 class FilePathCache : public testing::Test
@@ -73,9 +75,10 @@ protected:
     }
 
 protected:
-    NiceMock<MockFilePathStorage> mockStorage;
+    NiceMock<MockSqliteDatabase> mockDatabase;
+    NiceMock<MockFilePathStorage> mockStorage{mockDatabase};
     Cache cache{mockStorage};
-    NiceMock<MockFilePathStorage> mockStorageFilled;
+    NiceMock<MockFilePathStorage> mockStorageFilled{mockDatabase};
 };
 
 TEST_F(FilePathCache, FilePathIdWithOutAnyEntryCallDirectoryId)
@@ -379,7 +382,7 @@ TEST_F(FilePathCache, GetFileIdAfterAddFilePaths)
     Cache cacheFilled{mockStorageFilled};
 
     cacheFilled.addFilePaths(
-        {"/path3/to/file.h", "/path/to/file.h", "/path2/to/file2.h", "/path/to/file.cpp"});
+        FilePathViews{"/path3/to/file.h", "/path/to/file.h", "/path2/to/file2.h", "/path/to/file.cpp"});
 
     ASSERT_THAT(cacheFilled.filePath(101), Eq("/path3/to/file.h"));
 }
@@ -388,7 +391,7 @@ TEST_F(FilePathCache, GetFileIdAfterAddFilePathsWhichWasAlreadyAdded)
 {
     Cache cacheFilled{mockStorageFilled};
 
-    cacheFilled.addFilePaths({"/path3/to/file.h", "/path/to/file.h", "/path2/to/file2.h"});
+    cacheFilled.addFilePaths(FilePathViews{"/path3/to/file.h", "/path/to/file.h", "/path2/to/file2.h"});
 
     ASSERT_THAT(cacheFilled.filePath(42), Eq("/path/to/file.cpp"));
 }
@@ -398,13 +401,42 @@ TEST_F(FilePathCache, AddFilePathsCalls)
     Cache cacheFilled{mockStorageFilled};
     InSequence s;
 
+    EXPECT_CALL(mockDatabase, deferredBegin());
     EXPECT_CALL(mockStorageFilled, fetchDirectoryIdUnguarded(Eq("/path3/to"))).WillOnce(Return(7));
+    EXPECT_CALL(mockStorageFilled, fetchDirectoryIdUnguarded(Eq("/path/to"))).Times(0);
     EXPECT_CALL(mockStorageFilled, fetchSourceIdUnguarded(5, Eq("file.h"))).WillOnce(Return(99));
     EXPECT_CALL(mockStorageFilled, fetchSourceIdUnguarded(6, Eq("file2.h"))).WillOnce(Return(106));
     EXPECT_CALL(mockStorageFilled, fetchSourceIdUnguarded(7, Eq("file.h"))).WillOnce(Return(101));
+    EXPECT_CALL(mockStorageFilled, fetchSourceIdUnguarded(5, Eq("file.cpp"))).Times(0);
+    EXPECT_CALL(mockDatabase, commit());
 
     cacheFilled.addFilePaths(
-        {"/path3/to/file.h", "/path/to/file.h", "/path2/to/file2.h", "/path/to/file.cpp"});
+        FilePathViews{"/path3/to/file.h", "/path/to/file.h", "/path2/to/file2.h", "/path/to/file.cpp"});
 }
 
+TEST_F(FilePathCache, DontUseTransactionIfNotAddingFilesInAddFilePathsCalls)
+{
+    Cache cacheFilled{mockStorageFilled};
+    InSequence s;
+
+    EXPECT_CALL(mockDatabase, deferredBegin()).Times(0);
+    EXPECT_CALL(mockStorageFilled, fetchDirectoryIdUnguarded(Eq("/path/to"))).Times(0);
+    EXPECT_CALL(mockStorageFilled, fetchSourceIdUnguarded(5, Eq("file.cpp"))).Times(0);
+    EXPECT_CALL(mockDatabase, commit()).Times(0);
+
+    cacheFilled.addFilePaths(FilePathViews{"/path/to/file.cpp"});
+}
+
+TEST_F(FilePathCache, UseTransactionIfAddingFilesOnlyInAddFilePathsCalls)
+{
+    Cache cacheFilled{mockStorageFilled};
+    InSequence s;
+
+    EXPECT_CALL(mockDatabase, deferredBegin());
+    EXPECT_CALL(mockStorageFilled, fetchDirectoryIdUnguarded(Eq("/path/to"))).Times(0);
+    EXPECT_CALL(mockStorageFilled, fetchSourceIdUnguarded(5, Eq("file.h")));
+    EXPECT_CALL(mockDatabase, commit());
+
+    cacheFilled.addFilePaths(FilePathViews{"/path/to/file.h"});
+}
 } // namespace

tests/unit/unittest/mockfilepathcaching.h

@@ -42,5 +42,5 @@ public:
                        Utils::PathString(ClangBackEnd::DirectoryPathId directoryPathId));
     MOCK_CONST_METHOD1(directoryPathId,
                        ClangBackEnd::DirectoryPathId(ClangBackEnd::FilePathId filePathId));
+    MOCK_METHOD1(addFilePaths, void(const ClangBackEnd::FilePaths &filePaths));
 };
-

tests/unit/unittest/mockfilepathstorage.h

@@ -27,13 +27,18 @@
 
 #include "googletest.h"
 
+#include "mocksqlitedatabase.h"
+
 #include <filepathstoragesources.h>
 
 class MockFilePathStorage
 {
 public:
-    MOCK_METHOD1(fetchDirectoryId,
+    MockFilePathStorage(MockSqliteDatabase &mockDatabase)
-                 int (Utils::SmallStringView directoryPath));
+        : mockDatabase{mockDatabase}
+    {}
+
+    MOCK_METHOD1(fetchDirectoryId, int(Utils::SmallStringView directoryPath));
     MOCK_METHOD2(fetchSourceId,
                  int (int directoryId, Utils::SmallStringView sourceName));
     MOCK_METHOD1(fetchDirectoryIdUnguarded, int(Utils::SmallStringView directoryPath));
@@ -44,5 +49,9 @@ public:
                  ClangBackEnd::Sources::SourceNameAndDirectoryId (int sourceId));
     MOCK_METHOD0(fetchAllDirectories, std::vector<ClangBackEnd::Sources::Directory>());
     MOCK_METHOD0(fetchAllSources, std::vector<ClangBackEnd::Sources::Source>());
+
+    MockSqliteDatabase &database() { return mockDatabase; }
+
+    MockSqliteDatabase &mockDatabase;
 };
 

tests/unit/unittest/projectupdater-test.cpp

@@ -25,6 +25,7 @@
 
 #include "googletest.h"
 
+#include "mockfilepathcaching.h"
 #include "mockpchmanagerclient.h"
 #include "mockpchmanagernotifier.h"
 #include "mockpchmanagerserver.h"
@@ -470,6 +471,22 @@ TEST_F(ProjectUpdater, FetchProjectPartName)
     ASSERT_THAT(projectPartName, Eq(QString{" projectb"}));
 }
 
+TEST_F(ProjectUpdater, AddProjectFilesToFilePathCache)
+{
+    NiceMock<MockFilePathCaching> mockFilePathCaching;
+    ClangPchManager::ProjectUpdater updater{mockPchManagerServer,
+                                            mockFilePathCaching,
+                                            projectPartsStorage};
+
+    EXPECT_CALL(mockFilePathCaching,
+                addFilePaths(UnorderedElementsAre(Eq(headerPaths[0]),
+                                                  Eq(headerPaths[1]),
+                                                  Eq(sourcePaths[0]),
+                                                  Eq(sourcePaths[1]))));
+
+    updater.updateProjectParts({&projectPart}, {});
+}
+
 // test for update many time and get the same id
 
 } // namespace

tests/unit/unittest/stringcache-test.cpp

@@ -25,8 +25,9 @@
 
 #include "googletest.h"
 
-#include "mockmutex.h"
 #include "mockfilepathstorage.h"
+#include "mockmutex.h"
+#include "mocksqlitedatabase.h"
 
 #include <stringcache.h>
 
@@ -72,7 +73,8 @@ protected:
     }
 
 protected:
-    NiceMock<MockFilePathStorage> mockStorage;
+    NiceMock<MockSqliteDatabase> mockDatabase;
+    NiceMock<MockFilePathStorage> mockStorage{mockDatabase};
     StorageIdFunction mockStorageFetchDirectyId = [&] (Utils::SmallStringView string) {
         return mockStorage.fetchDirectoryId(string);
     };