ClangRefactoring: Remove not used pch files

Because we use UUID for pch files every time we rebuild a pch we get
a new file. This patch is collecting and removing this pch files.

Change-Id: Ia8688e7215ee7b41f39b2285323eaa345056bda3
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>
Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>

src/libs/clangsupport/filesystem.cpp

@@ -61,4 +61,10 @@ long long FileSystem::lastModified(FilePathId filePathId) const
     return 0;
 }
 
+void FileSystem::remove(const FilePathIds &filePathIds)
+{
+    for (FilePathId filePathId : filePathIds)
+        QFile::remove(QString{m_filePathCache.filePath(filePathId)});
+}
+
 } // namespace ClangBackEnd

src/libs/clangsupport/filesystem.h

@@ -41,6 +41,8 @@ public:
     FilePathIds directoryEntries(const QString &directoryPath) const override;
     long long lastModified(FilePathId filePathId) const override;
 
+    void remove(const FilePathIds &filePathIds) override;
+
 private:
     FilePathCachingInterface &m_filePathCache;
 };

src/libs/clangsupport/filesysteminterface.h

@@ -36,6 +36,7 @@ class FileSystemInterface
 public:
     virtual FilePathIds directoryEntries(const QString &directoryPath) const = 0;
     virtual long long lastModified(FilePathId filePathId) const = 0;
+    virtual void remove(const FilePathIds &filePathIds) = 0;
 
 protected:
     ~FileSystemInterface() = default;

src/tools/clangpchmanagerbackend/clangpchmanagerbackendmain.cpp

@@ -213,7 +213,9 @@ struct Data // because we have a cycle dependency
                                             pchCreationProgressCounter,
                                             preCompiledHeaderStorage,
                                             database,
-                                            environment};
+                                            environment,
+                                            fileSystem,
+                                            filePathCache};
     ClangBackEnd::PchTasksMerger pchTaskMerger{pchTaskQueue};
     ClangBackEnd::BuildDependenciesStorage<> buildDependencyStorage{database};
     ClangBackEnd::BuildDependencyCollector buildDependencyCollector{filePathCache,

src/tools/clangpchmanagerbackend/source/pchtaskqueue.cpp

@@ -26,6 +26,8 @@
 #include "pchtaskqueue.h"
 
 #include <environment.h>
+#include <filepathcaching.h>
+#include <filesystem.h>
 #include <pchcreatorinterface.h>
 #include <precompiledheaderstorageinterface.h>
 #include <progresscounter.h>
@@ -107,36 +109,67 @@ void PchTaskQueue::removePchTasks(const ProjectPartIds &projectsPartIds)
     removePchTasksByProjectPartId(projectsPartIds, m_projectPchTasks);
 }
 
-void PchTaskQueue::processProjectPchTasks()
+int PchTaskQueue::processProjectPchTasks()
 {
-    uint systemRunningTaskCount = m_systemPchTaskScheduler.slotUsage().used;
+    auto slotUsage = m_projectPchTaskScheduler.slotUsage();
+    uint freeTaskCount = slotUsage.free;
 
-    if (!systemRunningTaskCount) {
+    int newTaskCount = std::min<int>(int(freeTaskCount), int(m_projectPchTasks.size()));
-        uint freeTaskCount = m_projectPchTaskScheduler.slotUsage().free;
 
-        auto newEnd = std::prev(m_projectPchTasks.end(),
+    auto newEnd = std::prev(m_projectPchTasks.end(), newTaskCount);
-                                std::min<int>(int(freeTaskCount), int(m_projectPchTasks.size())));
     m_projectPchTaskScheduler.addTasks(createProjectTasks(
         {std::make_move_iterator(newEnd), std::make_move_iterator(m_projectPchTasks.end())}));
     m_projectPchTasks.erase(newEnd, m_projectPchTasks.end());
-    }
+
+    return newTaskCount + slotUsage.used;
 }
 
-void PchTaskQueue::processSystemPchTasks()
+int PchTaskQueue::processSystemPchTasks()
 {
-    uint freeTaskCount = m_systemPchTaskScheduler.slotUsage().free;
+    auto slotUsage = m_systemPchTaskScheduler.slotUsage();
+    uint freeTaskCount = slotUsage.free;
 
-    auto newEnd = std::prev(m_systemPchTasks.end(),
+    int newTaskCount = std::min<int>(int(freeTaskCount), int(m_systemPchTasks.size()));
-                            std::min<int>(int(freeTaskCount), int(m_systemPchTasks.size())));
+
+    auto newEnd = std::prev(m_systemPchTasks.end(), newTaskCount);
     m_systemPchTaskScheduler.addTasks(createSystemTasks(
         {std::make_move_iterator(newEnd), std::make_move_iterator(m_systemPchTasks.end())}));
     m_systemPchTasks.erase(newEnd, m_systemPchTasks.end());
+
+    return newTaskCount + slotUsage.used;
+}
+
+void PchTaskQueue::deleteUnusedPchs()
+{
+    FilePathIds existingPchFilePathIds = m_fileSystem.directoryEntries(
+        QString{m_environment.pchBuildDirectory()});
+    FilePathIds notAnymoreUsedPchFilePathIds;
+    notAnymoreUsedPchFilePathIds.reserve(existingPchFilePathIds.size());
+
+    FilePathIds usedPchFilePathIds = m_filePathCache.filePathIds(
+        m_precompiledHeaderStorage.fetchAllPchPaths());
+    std::sort(usedPchFilePathIds.begin(), usedPchFilePathIds.end());
+
+    std::set_difference(existingPchFilePathIds.begin(),
+                        existingPchFilePathIds.end(),
+                        usedPchFilePathIds.begin(),
+                        usedPchFilePathIds.end(),
+                        std::back_inserter(notAnymoreUsedPchFilePathIds));
+
+    m_fileSystem.remove(notAnymoreUsedPchFilePathIds);
 }
 
 void PchTaskQueue::processEntries()
 {
-    processSystemPchTasks();
+    int projectTaskCount = 0;
-    processProjectPchTasks();
+    int systemTaskCount = processSystemPchTasks();
+    if (systemTaskCount == 0)
+        projectTaskCount = processProjectPchTasks();
+
+    int totalTaskCount = projectTaskCount + systemTaskCount;
+
+    if (totalTaskCount == 0)
+        deleteUnusedPchs();
 }
 
 std::vector<PchTaskQueue::Task> PchTaskQueue::createProjectTasks(PchTasks &&pchTasks) const

src/tools/clangpchmanagerbackend/source/pchtaskqueue.h

@@ -37,6 +37,8 @@ class PchCreatorInterface;
 class PrecompiledHeaderStorageInterface;
 class ProgressCounter;
 class Environment;
+class FileSystemInterface;
+class FilePathCachingInterface;
 
 class PchTaskQueue final : public PchTaskQueueInterface
 {
@@ -48,13 +50,17 @@ public:
                  ProgressCounter &progressCounter,
                  PrecompiledHeaderStorageInterface &precompiledHeaderStorage,
                  Sqlite::TransactionInterface &transactionsInterface,
-                 const Environment &environment)
+                 const Environment &environment,
+                 FileSystemInterface &fileSystem,
+                 FilePathCachingInterface &filePathCache)
         : m_systemPchTaskScheduler(systemPchTaskScheduler)
         , m_projectPchTaskScheduler(projectPchTaskScheduler)
         , m_precompiledHeaderStorage(precompiledHeaderStorage)
         , m_transactionsInterface(transactionsInterface)
         , m_progressCounter(progressCounter)
         , m_environment(environment)
+        , m_fileSystem(fileSystem)
+        , m_filePathCache(filePathCache)
     {}
 
     void addSystemPchTasks(PchTasks &&pchTasks) override;
@@ -72,8 +78,9 @@ public:
 private:
     void addPchTasks(PchTasks &&pchTasks, PchTasks &destination);
     void removePchTasksByProjectPartId(const ProjectPartIds &projectsPartIds, PchTasks &destination);
-    void processProjectPchTasks();
+    int processProjectPchTasks();
-    void processSystemPchTasks();
+    int processSystemPchTasks();
+    void deleteUnusedPchs();
 
 private:
     PchTasks m_systemPchTasks;
@@ -84,6 +91,8 @@ private:
     Sqlite::TransactionInterface &m_transactionsInterface;
     ProgressCounter &m_progressCounter;
     const Environment &m_environment;
+    FileSystemInterface &m_fileSystem;
+    FilePathCachingInterface &m_filePathCache;
 };
 
 } // namespace ClangBackEnd

src/tools/clangpchmanagerbackend/source/precompiledheaderstorage.h

@@ -219,6 +219,22 @@ public:
         return {};
     }
 
+    FilePaths fetchAllPchPaths() const
+    {
+        try {
+            Sqlite::DeferredTransaction transaction{database};
+
+            auto filePaths = fetchAllPchPathsStatement.template values<FilePath>(1024);
+
+            transaction.commit();
+
+            return filePaths;
+
+        } catch (const Sqlite::StatementIsBusy) {
+            return fetchAllPchPaths();
+        }
+    }
+
 public:
     Sqlite::ImmediateNonThrowingDestructorTransaction transaction;
     Database &database;
@@ -262,6 +278,10 @@ public:
         "SELECT projectPchBuildTime, systemPchBuildTime FROM precompiledHeaders WHERE "
         "projectPartId = ?",
         database};
+    mutable ReadStatement fetchAllPchPathsStatement{
+        "SELECT DISTINCT systemPchPath FROM precompiledHeaders UNION ALL SELECT "
+        "DISTINCT projectPchPath FROM precompiledHeaders",
+        database};
 };
 
 }

src/tools/clangpchmanagerbackend/source/precompiledheaderstorageinterface.h

@@ -60,6 +60,7 @@ public:
     virtual FilePath fetchPrecompiledHeader(ProjectPartId projectPartId) const = 0;
     virtual PchPaths fetchPrecompiledHeaders(ProjectPartId projectPartId) const = 0;
     virtual PrecompiledHeaderTimeStamps fetchTimeStamps(ProjectPartId projectPartId) const = 0;
+    virtual FilePaths fetchAllPchPaths() const = 0;
 
 protected:
     ~PrecompiledHeaderStorageInterface() = default;

tests/unit/unittest/mockfilesystem.h

@@ -34,4 +34,5 @@ class MockFileSystem : public ClangBackEnd::FileSystemInterface
 public:
     MOCK_CONST_METHOD1(directoryEntries, ClangBackEnd::FilePathIds(const QString &directoryPath));
     MOCK_CONST_METHOD1(lastModified, long long(ClangBackEnd::FilePathId filePathId));
+    MOCK_METHOD1(remove, void(const ClangBackEnd::FilePathIds &filePathIds));
 };

tests/unit/unittest/mockprecompiledheaderstorage.h

@@ -57,4 +57,5 @@ public:
     MOCK_CONST_METHOD1(
         fetchTimeStamps,
         ClangBackEnd::PrecompiledHeaderTimeStamps(ClangBackEnd::ProjectPartId projectPartId));
+    MOCK_CONST_METHOD0(fetchAllPchPaths, ClangBackEnd::FilePaths());
 };

tests/unit/unittest/mocksqlitereadstatement.cpp

@@ -107,6 +107,12 @@ FilePathIds MockSqliteReadStatement::values<ClangBackEnd::FilePathId>(std::size_
     return valuesReturnFilePathIds(reserveSize, projectPartId);
 }
 
+template<>
+ClangBackEnd::FilePaths MockSqliteReadStatement::values<ClangBackEnd::FilePath>(std::size_t reserveSize)
+{
+    return valuesReturnFilePaths(reserveSize);
+}
+
 template <>
 std::vector<Sources::Directory> MockSqliteReadStatement::values<Sources::Directory, 2>(std::size_t reserveSize)
 {

tests/unit/unittest/mocksqlitereadstatement.h

@@ -113,6 +113,8 @@ public:
 
     MOCK_METHOD1(valueReturnFilePath, Utils::optional<ClangBackEnd::FilePath>(int));
 
+    MOCK_METHOD1(valuesReturnFilePaths, ClangBackEnd::FilePaths(std::size_t));
+
     MOCK_METHOD1(valueReturnSmallString,
                  Utils::optional<Utils::SmallString>(int));
 
@@ -233,6 +235,9 @@ template<>
 FilePathIds MockSqliteReadStatement::values<ClangBackEnd::FilePathId>(std::size_t reserveSize,
                                                                       const int &projectPartId);
 
+template<>
+ClangBackEnd::FilePaths MockSqliteReadStatement::values<ClangBackEnd::FilePath>(std::size_t reserveSize);
+
 template <>
 std::vector<Sources::Directory> MockSqliteReadStatement::values<Sources::Directory, 2>(std::size_t reserveSize);
 

tests/unit/unittest/pchtaskqueue-test.cpp

@@ -25,14 +25,18 @@
 
 #include "googletest.h"
 
+#include "mockfilesystem.h"
 #include "mockpchcreator.h"
 #include "mockprecompiledheaderstorage.h"
 #include "mocksqlitetransactionbackend.h"
 #include "mocktaskscheduler.h"
 #include "testenvironment.h"
 
+#include <filepathcaching.h>
 #include <pchtaskqueue.h>
 #include <progresscounter.h>
+#include <refactoringdatabaseinitializer.h>
+#include <sqlitedatabase.h>
 
 namespace {
 
@@ -45,9 +49,26 @@ using ClangBackEnd::SlotUsage;
 class PchTaskQueue : public testing::Test
 {
 protected:
+    ClangBackEnd::FilePathId filePathId(Utils::SmallStringView path)
+    {
+        return filePathCache.filePathId(ClangBackEnd::FilePathView{path});
+    }
+
+    ClangBackEnd::FilePathIds filePathIds(const Utils::PathStringVector &paths)
+    {
+        return filePathCache.filePathIds(Utils::transform(paths, [](const Utils::PathString &path) {
+            return ClangBackEnd::FilePathView(path);
+        }));
+    }
+
+protected:
+    Sqlite::Database database{":memory:", Sqlite::JournalMode::Memory};
+    ClangBackEnd::RefactoringDatabaseInitializer<Sqlite::Database> initializer{database};
+    ClangBackEnd::FilePathCaching filePathCache{database};
     NiceMock<MockTaskScheduler<ClangBackEnd::PchTaskQueue::Task>> mockSytemPchTaskScheduler;
     NiceMock<MockTaskScheduler<ClangBackEnd::PchTaskQueue::Task>> mockProjectPchTaskScheduler;
     NiceMock<MockPrecompiledHeaderStorage> mockPrecompiledHeaderStorage;
+    NiceMock<MockFileSystem> mockFileSystem;
     MockSqliteTransactionBackend mockSqliteTransactionBackend;
     NiceMock<MockFunction<void(int, int)>> mockSetProgressCallback;
     ClangBackEnd::ProgressCounter progressCounter{mockSetProgressCallback.AsStdFunction()};
@@ -57,7 +78,9 @@ protected:
                                      progressCounter,
                                      mockPrecompiledHeaderStorage,
                                      mockSqliteTransactionBackend,
-                                     testEnvironment};
+                                     testEnvironment,
+                                     mockFileSystem,
+                                     filePathCache};
     IncludeSearchPaths systemIncludeSearchPaths{
         {"/includes", 1, IncludeSearchPathType::BuiltIn},
         {"/other/includes", 2, IncludeSearchPathType::System}};
@@ -390,4 +413,91 @@ TEST_F(PchTaskQueue, DeleteSystemPchEntryInDatabaseIfNoPchIsGenerated)
     tasks.front()(mockPchCreator);
 }
 
+TEST_F(PchTaskQueue, DontDeleteUnusedPchsIfSystemTaskAreProcessed)
+{
+    QString pchsDirectory{testEnvironment.pchBuildDirectory()};
+    ON_CALL(mockSytemPchTaskScheduler, slotUsage()).WillByDefault(Return(SlotUsage{2, 1}));
+    ON_CALL(mockProjectPchTaskScheduler, slotUsage()).WillByDefault(Return(SlotUsage{2, 0}));
+    ON_CALL(mockFileSystem, directoryEntries(Eq(pchsDirectory)))
+        .WillByDefault(Return(filePathIds({"/tmp/foo", "/tmp/bar"})));
+    ON_CALL(mockPrecompiledHeaderStorage, fetchAllPchPaths())
+        .WillByDefault(Return(ClangBackEnd::FilePaths{"/tmp/foo", "/tmp/poo"}));
+
+    EXPECT_CALL(mockFileSystem, remove(_)).Times(0);
+
+    queue.processEntries();
+}
+
+TEST_F(PchTaskQueue, DontDeleteUnusedPchsIfProjectTaskAreProcessed)
+{
+    QString pchsDirectory{testEnvironment.pchBuildDirectory()};
+    ON_CALL(mockSytemPchTaskScheduler, slotUsage()).WillByDefault(Return(SlotUsage{2, 0}));
+    ON_CALL(mockProjectPchTaskScheduler, slotUsage()).WillByDefault(Return(SlotUsage{2, 1}));
+    ON_CALL(mockFileSystem, directoryEntries(Eq(pchsDirectory)))
+        .WillByDefault(Return(filePathIds({"/tmp/foo", "/tmp/bar"})));
+    ON_CALL(mockPrecompiledHeaderStorage, fetchAllPchPaths())
+        .WillByDefault(Return(ClangBackEnd::FilePaths{"/tmp/foo", "/tmp/poo"}));
+
+    EXPECT_CALL(mockFileSystem, remove(_)).Times(0);
+
+    queue.processEntries();
+}
+
+TEST_F(PchTaskQueue, DontDeleteUnusedPchsIfSystemTaskIsAdded)
+{
+    QString pchsDirectory{testEnvironment.pchBuildDirectory()};
+    ON_CALL(mockSytemPchTaskScheduler, slotUsage()).WillByDefault(Return(SlotUsage{2, 0}));
+    ON_CALL(mockProjectPchTaskScheduler, slotUsage()).WillByDefault(Return(SlotUsage{2, 0}));
+    ON_CALL(mockFileSystem, directoryEntries(Eq(pchsDirectory)))
+        .WillByDefault(Return(filePathIds({"/tmp/foo", "/tmp/bar"})));
+    ON_CALL(mockPrecompiledHeaderStorage, fetchAllPchPaths())
+        .WillByDefault(Return(ClangBackEnd::FilePaths{"/tmp/foo", "/tmp/poo"}));
+    queue.addSystemPchTasks({systemTask1});
+
+    EXPECT_CALL(mockFileSystem, remove(_)).Times(0);
+
+    queue.processEntries();
+}
+
+TEST_F(PchTaskQueue, DontDeleteUnusedPchsIfProjectTaskIsAdded)
+{
+    QString pchsDirectory{testEnvironment.pchBuildDirectory()};
+    ON_CALL(mockSytemPchTaskScheduler, slotUsage()).WillByDefault(Return(SlotUsage{2, 0}));
+    ON_CALL(mockProjectPchTaskScheduler, slotUsage()).WillByDefault(Return(SlotUsage{2, 0}));
+    ON_CALL(mockFileSystem, directoryEntries(Eq(pchsDirectory)))
+        .WillByDefault(Return(filePathIds({"/tmp/foo", "/tmp/bar"})));
+    ON_CALL(mockPrecompiledHeaderStorage, fetchAllPchPaths())
+        .WillByDefault(Return(ClangBackEnd::FilePaths{"/tmp/foo", "/tmp/poo"}));
+    queue.addProjectPchTasks({projectTask1});
+
+    EXPECT_CALL(mockFileSystem, remove(_)).Times(0);
+
+    queue.processEntries();
+}
+
+TEST_F(PchTaskQueue, DeleteUnusedPchs)
+{
+    QString pchsDirectory{testEnvironment.pchBuildDirectory()};
+    ON_CALL(mockSytemPchTaskScheduler, slotUsage()).WillByDefault(Return(SlotUsage{2, 0}));
+    ON_CALL(mockProjectPchTaskScheduler, slotUsage()).WillByDefault(Return(SlotUsage{2, 0}));
+    ON_CALL(mockFileSystem, directoryEntries(Eq(pchsDirectory)))
+        .WillByDefault(Return(filePathIds({
+            "/tmp/foo",
+            "/tmp/bar",
+            "/tmp/hoo",
+            "/tmp/too",
+        })));
+    ON_CALL(mockPrecompiledHeaderStorage, fetchAllPchPaths())
+        .WillByDefault(Return(ClangBackEnd::FilePaths{
+            "/tmp/foo",
+            "/tmp/poo",
+            "/tmp/too",
+        }));
+
+    EXPECT_CALL(mockFileSystem,
+                remove(UnorderedElementsAre(filePathId("/tmp/bar"), filePathId("/tmp/hoo"))));
+
+    queue.processEntries();
+}
+
 } // namespace

tests/unit/unittest/precompiledheaderstorage-test.cpp

@@ -53,6 +53,7 @@ protected:
     MockSqliteReadStatement &fetchPrecompiledHeaderStatement = storage.fetchPrecompiledHeaderStatement;
     MockSqliteReadStatement &fetchPrecompiledHeadersStatement = storage.fetchPrecompiledHeadersStatement;
     MockSqliteReadStatement &fetchTimeStampsStatement = storage.fetchTimeStampsStatement;
+    MockSqliteReadStatement &fetchAllPchPathsStatement = storage.fetchAllPchPathsStatement;
 };
 
 TEST_F(PrecompiledHeaderStorage, UseTransaction)
@@ -458,6 +459,32 @@ TEST_F(PrecompiledHeaderStorage, FetchTimeStampsBusy)
     storage.fetchTimeStamps(23);
 }
 
+TEST_F(PrecompiledHeaderStorage, FetchAllPchPaths)
+{
+    InSequence s;
+
+    EXPECT_CALL(database, deferredBegin());
+    EXPECT_CALL(fetchAllPchPathsStatement, valuesReturnFilePaths(_));
+    EXPECT_CALL(database, commit());
+
+    storage.fetchAllPchPaths();
+}
+
+TEST_F(PrecompiledHeaderStorage, FetchAllPchPathsIsBusy)
+{
+    InSequence s;
+
+    EXPECT_CALL(database, deferredBegin());
+    EXPECT_CALL(fetchAllPchPathsStatement, valuesReturnFilePaths(_))
+        .WillOnce(Throw(Sqlite::StatementIsBusy{""}));
+    EXPECT_CALL(database, rollback());
+    EXPECT_CALL(database, deferredBegin());
+    EXPECT_CALL(fetchAllPchPathsStatement, valuesReturnFilePaths(_));
+    EXPECT_CALL(database, commit());
+
+    storage.fetchAllPchPaths();
+}
+
 class PrecompiledHeaderStorageSlowTest : public testing::Test
 {
 protected:
@@ -478,4 +505,18 @@ TEST_F(PrecompiledHeaderStorageSlowTest, NoFetchTimeStamps)
                       Field(&ClangBackEnd::PrecompiledHeaderTimeStamps::system, Eq(33))));
 }
 
+TEST_F(PrecompiledHeaderStorageSlowTest, FetchAllPchPaths)
+{
+    storage.insertProjectPrecompiledHeader(11, "/tmp/yi", 22);
+    storage.insertProjectPrecompiledHeader(12, "/tmp/er", 22);
+    storage.insertSystemPrecompiledHeaders({11, 12}, "/tmp/se", 33);
+    storage.insertSystemPrecompiledHeaders({13}, "/tmp/wu", 33);
+    storage.insertProjectPrecompiledHeader(13, "/tmp/san", 22);
+
+    auto filePathIds = storage.fetchAllPchPaths();
+
+    ASSERT_THAT(filePathIds,
+                UnorderedElementsAre("/tmp/er", "/tmp/san", "/tmp/se", "/tmp/wu", "/tmp/yi"));
+}
+
 } // namespace

tests/unit/unittest/unittest.pro

@@ -39,7 +39,7 @@ CONFIG(release, debug|release):QMAKE_LFLAGS += -Wl,--strip-debug
 }
 
 gcc:!clang: QMAKE_CXXFLAGS += -Wno-noexcept-type
-msvc: QMAKE_CXXFLAGS += /bigobj /wd4267 /wd4141 /wd4146
+msvc: QMAKE_CXXFLAGS += /bigobj /wd4267 /wd4141 /wd4146 /wd4624
 
 # create fake CppTools.json for the mime type definitions
 dependencyList = "\"Dependencies\" : []"