diff --git a/src/libs/clangsupport/filepathcache.h b/src/libs/clangsupport/filepathcache.h index 2faef6e5037..9a90d71497d 100644 --- a/src/libs/clangsupport/filepathcache.h +++ b/src/libs/clangsupport/filepathcache.h @@ -161,6 +161,27 @@ public: return m_fileNameCache.string(filePathId.filePathId, fetchSoureNameAndDirectoryId).directoryId; } + void addFilePaths(FilePathViews &&filePaths) + { + auto directoryPaths = Utils::transform>( + filePaths, [](FilePathView filePath) { return filePath.directory(); }); + + m_directoryPathCache.addStrings(std::move(directoryPaths), + [&](Utils::SmallStringView directoryPath) { + return m_filePathStorage.fetchDirectoryIdUnguarded( + directoryPath); + }); + + auto sourcePaths = Utils::transform>(filePaths, [&](FilePathView filePath) { + return FileNameView{filePath.name(), m_directoryPathCache.stringId(filePath.directory())}; + }); + + m_fileNameCache.addStrings(std::move(sourcePaths), [&](FileNameView fileNameView) { + return m_filePathStorage.fetchSourceIdUnguarded(fileNameView.directoryId, + fileNameView.fileName); + }); + } + private: mutable DirectoryPathCache m_directoryPathCache; mutable FileNameCache m_fileNameCache; diff --git a/src/libs/clangsupport/filepathstorage.h b/src/libs/clangsupport/filepathstorage.h index 8434f506597..3d6ca792dec 100644 --- a/src/libs/clangsupport/filepathstorage.h +++ b/src/libs/clangsupport/filepathstorage.h @@ -48,6 +48,16 @@ public: : m_statementFactory(statementFactory) {} + int fetchDirectoryIdUnguarded(Utils::SmallStringView directoryPath) + { + Utils::optional optionalDirectoryId = readDirectoryId(directoryPath); + + if (optionalDirectoryId) + return optionalDirectoryId.value(); + + return writeDirectoryId(directoryPath); + } + int fetchDirectoryId(Utils::SmallStringView directoryPath) { try { @@ -125,19 +135,22 @@ public: } } + int fetchSourceIdUnguarded(int directoryId, Utils::SmallStringView sourceName) + { + Utils::optional optionalSourceId = readSourceId(directoryId, sourceName); + + if (optionalSourceId) + return optionalSourceId.value(); + + return writeSourceId(directoryId, sourceName); + } + int fetchSourceId(int directoryId, Utils::SmallStringView sourceName) { try { Sqlite::DeferredTransaction transaction{m_statementFactory.database}; - Utils::optional optionalSourceId = readSourceId(directoryId, sourceName); - - int sourceId = -1; - - if (optionalSourceId) - sourceId = optionalSourceId.value(); - else - sourceId = writeSourceId(directoryId, sourceName); + int sourceId = fetchSourceIdUnguarded(directoryId, sourceName); transaction.commit(); diff --git a/src/libs/clangsupport/stringcache.h b/src/libs/clangsupport/stringcache.h index 5a1ea8fd354..471d023b900 100644 --- a/src/libs/clangsupport/stringcache.h +++ b/src/libs/clangsupport/stringcache.h @@ -26,6 +26,7 @@ #pragma once #include "filepathstoragesources.h" +#include "set_algorithm.h" #include "stringcachealgorithms.h" #include "stringcachefwd.h" @@ -152,9 +153,7 @@ public: void uncheckedPopulate(CacheEntries &&entries) { - std::sort(entries.begin(), - entries.end(), - [] (StringViewType first, StringViewType second) { + std::sort(entries.begin(), entries.end(), [](StringViewType first, StringViewType second) { return compare(first, second) < 0; }); @@ -173,9 +172,59 @@ public: m_indices.resize(max_id, -1); - auto begin = m_strings.cbegin(); - for (auto current = begin; current != m_strings.end(); ++current) - m_indices[current->id] = std::distance(begin, current); + updateIndices(); + } + + template + void addStrings(std::vector &&strings, Function storageFunction) + { + auto less = [](StringViewType first, StringViewType second) { + return compare(first, second) < 0; + }; + + std::sort(strings.begin(), strings.end(), less); + + strings.erase(std::unique(strings.begin(), strings.end()), strings.end()); + + CacheEntries newCacheEntries; + newCacheEntries.reserve(strings.size()); + + std::set_difference(strings.begin(), + strings.end(), + m_strings.begin(), + m_strings.end(), + make_iterator([&](StringViewType newString) { + IndexType index = storageFunction(newString); + newCacheEntries.emplace_back(newString, index); + }), + less); + + if (newCacheEntries.size()) { + auto found = std::max_element(newCacheEntries.begin(), + newCacheEntries.end(), + [](const auto &first, const auto &second) { + return first.id < second.id; + }); + + int max_id = found->id + 1; + + if (max_id > int(m_indices.size())) + m_indices.resize(max_id, -1); + + CacheEntries mergedCacheEntries; + mergedCacheEntries.reserve(newCacheEntries.size() + m_strings.size()); + + std::merge(std::make_move_iterator(m_strings.begin()), + std::make_move_iterator(m_strings.end()), + std::make_move_iterator(newCacheEntries.begin()), + std::make_move_iterator(newCacheEntries.end()), + std::back_inserter(mergedCacheEntries), + less); + + m_strings = std::move(mergedCacheEntries); + + updateIndices(); + } } IndexType stringId(StringViewType stringView) @@ -291,6 +340,13 @@ public: } private: + void updateIndices() + { + auto begin = m_strings.cbegin(); + for (auto current = begin; current != m_strings.end(); ++current) + m_indices[current->id] = std::distance(begin, current); + } + Found find(StringViewType stringView) { return findInSorted(m_strings.cbegin(), m_strings.cend(), stringView, compare); diff --git a/src/libs/utils/smallstring.h b/src/libs/utils/smallstring.h index e473378ffe3..6708560ac80 100644 --- a/src/libs/utils/smallstring.h +++ b/src/libs/utils/smallstring.h @@ -181,7 +181,7 @@ public: } BasicSmallString(BasicSmallString &&other) noexcept - : m_data(other.m_data) + : m_data(std::move(other.m_data)) { other.m_data.reset(); } @@ -193,8 +193,8 @@ public: this->~BasicSmallString(); - m_data = other.m_data; - other.m_data = Internal::StringDataLayout(); + m_data = std::move(other.m_data); + other.m_data.reset(); return *this; } diff --git a/tests/unit/unittest/filepathcache-test.cpp b/tests/unit/unittest/filepathcache-test.cpp index 8ec44064e43..c46bb0ce880 100644 --- a/tests/unit/unittest/filepathcache-test.cpp +++ b/tests/unit/unittest/filepathcache-test.cpp @@ -54,8 +54,7 @@ protected: .WillByDefault(Return(63)); ON_CALL(mockStorage, fetchSourceId(6, Eq("file.cpp"))) .WillByDefault(Return(72)); - ON_CALL(mockStorage, fetchDirectoryPath(5)) - .WillByDefault(Return(Utils::PathString("/path/to"))); + ON_CALL(mockStorage, fetchDirectoryPath(5)).WillByDefault(Return(Utils::PathString("/path/to"))); ON_CALL(mockStorage, fetchSourceNameAndDirectoryId(42)) .WillByDefault(Return(SourceNameAndDirectoryId("file.cpp", 5))); ON_CALL(mockStorageFilled, fetchAllSources()) @@ -67,6 +66,10 @@ protected: ON_CALL(mockStorageFilled, fetchAllDirectories()) .WillByDefault(Return( std::vector({{"/path2/to", 6}, {"/path/to", 5}}))); + ON_CALL(mockStorageFilled, fetchDirectoryIdUnguarded(Eq("/path3/to"))).WillByDefault(Return(7)); + ON_CALL(mockStorageFilled, fetchSourceIdUnguarded(7, Eq("file.h"))).WillByDefault(Return(101)); + ON_CALL(mockStorageFilled, fetchSourceIdUnguarded(6, Eq("file2.h"))).WillByDefault(Return(106)); + ON_CALL(mockStorageFilled, fetchSourceIdUnguarded(5, Eq("file.h"))).WillByDefault(Return(99)); } protected: @@ -370,4 +373,38 @@ TEST_F(FilePathCache, GetFilePathInFilledCache) ASSERT_THAT(path, Eq("/path/to/file.cpp")); } + +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"}); + + ASSERT_THAT(cacheFilled.filePath(101), Eq("/path3/to/file.h")); +} + +TEST_F(FilePathCache, GetFileIdAfterAddFilePathsWhichWasAlreadyAdded) +{ + Cache cacheFilled{mockStorageFilled}; + + cacheFilled.addFilePaths({"/path3/to/file.h", "/path/to/file.h", "/path2/to/file2.h"}); + + ASSERT_THAT(cacheFilled.filePath(42), Eq("/path/to/file.cpp")); +} + +TEST_F(FilePathCache, AddFilePathsCalls) +{ + Cache cacheFilled{mockStorageFilled}; + InSequence s; + + EXPECT_CALL(mockStorageFilled, fetchDirectoryIdUnguarded(Eq("/path3/to"))).WillOnce(Return(7)); + 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)); + + cacheFilled.addFilePaths( + {"/path3/to/file.h", "/path/to/file.h", "/path2/to/file2.h", "/path/to/file.cpp"}); +} + } // namespace diff --git a/tests/unit/unittest/filepathstorage-test.cpp b/tests/unit/unittest/filepathstorage-test.cpp index 309b5d0c705..e943ab43dad 100644 --- a/tests/unit/unittest/filepathstorage-test.cpp +++ b/tests/unit/unittest/filepathstorage-test.cpp @@ -599,4 +599,75 @@ TEST_F(FilePathStorage, FetchDirectoryIdCallsThrows) ASSERT_ANY_THROW(storage.fetchDirectoryId(41)); } +TEST_F(FilePathStorage, GetTheDirectoryIdBackAfterFetchingANewEntryFromDirectoriesUnguarded) +{ + auto directoryId = storage.fetchDirectoryIdUnguarded("/some/not/known/path"); + + ASSERT_THAT(directoryId, 12); +} + +TEST_F(FilePathStorage, GetTheSourceIdBackAfterFetchingANewEntryFromSourcesUnguarded) +{ + auto sourceId = storage.fetchSourceIdUnguarded(5, "unknownfile.h"); + + ASSERT_THAT(sourceId, 12); +} + +TEST_F(FilePathStorage, CallSelectForFetchingDirectoryIdForKnownPathUnguarded) +{ + InSequence s; + + EXPECT_CALL(selectDirectoryIdFromDirectoriesByDirectoryPath, + valueReturnInt32(TypedEq("/path/to"))); + + storage.fetchDirectoryIdUnguarded("/path/to"); +} + +TEST_F(FilePathStorage, CallSelectForFetchingSourceIdForKnownPathUnguarded) +{ + InSequence s; + + EXPECT_CALL(selectSourceIdFromSourcesByDirectoryIdAndSourceName, + valueReturnInt32(5, Eq("file.h"))); + + storage.fetchSourceIdUnguarded(5, "file.h"); +} + +TEST_F(FilePathStorage, CallNotWriteForFetchingDirectoryIdForKnownPathUnguarded) +{ + EXPECT_CALL(insertIntoDirectories, write(An())).Times(0); + + storage.fetchDirectoryIdUnguarded("/path/to"); +} + +TEST_F(FilePathStorage, CallNotWriteForFetchingSoureIdForKnownEntryUnguarded) +{ + EXPECT_CALL(insertIntoSources, write(An(), An())).Times(0); + + storage.fetchSourceIdUnguarded(5, "file.h"); +} + +TEST_F(FilePathStorage, CallSelectAndWriteForFetchingDirectoryIdForUnknownPathUnguarded) +{ + InSequence s; + + EXPECT_CALL(selectDirectoryIdFromDirectoriesByDirectoryPath, + valueReturnInt32(TypedEq("/some/not/known/path"))); + EXPECT_CALL(insertIntoDirectories, write(TypedEq("/some/not/known/path"))); + + storage.fetchDirectoryIdUnguarded("/some/not/known/path"); +} + +TEST_F(FilePathStorage, CallSelectAndWriteForFetchingSourceIdForUnknownEntryUnguarded) +{ + InSequence s; + + EXPECT_CALL(selectSourceIdFromSourcesByDirectoryIdAndSourceName, + valueReturnInt32(5, Eq("unknownfile.h"))); + EXPECT_CALL(insertIntoSources, + write(TypedEq(5), TypedEq("unknownfile.h"))); + + storage.fetchSourceIdUnguarded(5, "unknownfile.h"); +} + } // namespace diff --git a/tests/unit/unittest/mockfilepathstorage.h b/tests/unit/unittest/mockfilepathstorage.h index 4adb965e868..c6701228e72 100644 --- a/tests/unit/unittest/mockfilepathstorage.h +++ b/tests/unit/unittest/mockfilepathstorage.h @@ -36,6 +36,8 @@ public: int (Utils::SmallStringView directoryPath)); MOCK_METHOD2(fetchSourceId, int (int directoryId, Utils::SmallStringView sourceName)); + MOCK_METHOD1(fetchDirectoryIdUnguarded, int(Utils::SmallStringView directoryPath)); + MOCK_METHOD2(fetchSourceIdUnguarded, int(int directoryId, Utils::SmallStringView sourceName)); MOCK_METHOD1(fetchDirectoryPath, Utils::PathString (int directoryId)); MOCK_METHOD1(fetchSourceNameAndDirectoryId, diff --git a/tests/unit/unittest/stringcache-test.cpp b/tests/unit/unittest/stringcache-test.cpp index f8c7114e5eb..027adb2ac61 100644 --- a/tests/unit/unittest/stringcache-test.cpp +++ b/tests/unit/unittest/stringcache-test.cpp @@ -55,7 +55,21 @@ using CacheEntries = Cache::CacheEntries; class StringCache : public testing::Test { protected: - void SetUp(); + void SetUp() + { + std::sort(filePaths.begin(), filePaths.end(), [](auto &f, auto &l) { + return compare(f, l) < 0; + }); + std::sort(reverseFilePaths.begin(), reverseFilePaths.end(), [](auto &f, auto &l) { + return reverseCompare(f, l) < 0; + }); + + ON_CALL(mockStorage, fetchDirectoryId(Eq("foo"))).WillByDefault(Return(42)); + ON_CALL(mockStorage, fetchDirectoryId(Eq("bar"))).WillByDefault(Return(43)); + ON_CALL(mockStorage, fetchDirectoryId(Eq("poo"))).WillByDefault(Return(44)); + ON_CALL(mockStorage, fetchDirectoryId(Eq("taa"))).WillByDefault(Return(45)); + ON_CALL(mockStorage, fetchDirectoryPath(41)).WillByDefault(Return(Utils::PathString("bar"))); + } protected: NiceMock mockStorage; @@ -490,15 +504,57 @@ TEST_F(StringCache, FetchDirectoryPathForUnknownIndex) ASSERT_THAT(string, Eq("bar")); } -void StringCache::SetUp() +TEST_F(StringCache, AddStringCalls) { - std::sort(filePaths.begin(), filePaths.end(), [] (auto &f, auto &l) { return compare(f, l) < 0;}); - std::sort(reverseFilePaths.begin(), reverseFilePaths.end(), [] (auto &f, auto &l) { return reverseCompare(f, l) < 0;}); + EXPECT_CALL(mockStorage, fetchDirectoryId(Eq("foo"))); + EXPECT_CALL(mockStorage, fetchDirectoryId(Eq("bar"))); + EXPECT_CALL(mockStorage, fetchDirectoryId(Eq("poo"))); - ON_CALL(mockStorage, fetchDirectoryId(Eq("foo"))) - .WillByDefault(Return(42)); - ON_CALL(mockStorage, fetchDirectoryPath(41)) - .WillByDefault(Return(Utils::PathString("bar"))); -} + cache.addStrings({"foo", "bar", "poo"}, mockStorageFetchDirectyId); } +TEST_F(StringCache, AddStringCallsOnlyForNewStrings) +{ + cache.addStrings({"foo", "poo"}, mockStorageFetchDirectyId); + + EXPECT_CALL(mockStorage, fetchDirectoryId(Eq("taa"))); + EXPECT_CALL(mockStorage, fetchDirectoryId(Eq("bar"))); + + cache.addStrings({"foo", "bar", "poo", "taa"}, mockStorageFetchDirectyId); +} + +TEST_F(StringCache, GetStringIdAfterAddingStrings) +{ + cache.addStrings({"foo", "bar", "poo", "taa"}, mockStorageFetchDirectyId); + + ASSERT_THAT(cache.string(cache.stringId("taa")), Eq("taa")); +} + +TEST_F(StringCache, GetStringAfterAddingStrings) +{ + cache.addStrings({"foo", "bar", "poo", "taa"}, mockStorageFetchDirectyId); + + ASSERT_THAT(cache.string(cache.stringId("taa")), Eq("taa")); +} + +TEST_F(StringCache, GetStringIdAfterAddingStringsMultipleTimes) +{ + cache.addStrings({"foo", "taa"}, mockStorageFetchDirectyId); + + cache.addStrings({"foo", "bar", "poo", "taa"}, mockStorageFetchDirectyId); + + ASSERT_THAT(cache.string(cache.stringId("taa")), Eq("taa")); +} + +TEST_F(StringCache, GetStringIdAfterAddingTheSameStringsMultipleTimes) +{ + cache.addStrings({"foo", "taa", "poo", "taa", "bar", "taa"}, mockStorageFetchDirectyId); + + ASSERT_THAT(cache.string(cache.stringId("taa")), Eq("taa")); +} + +TEST_F(StringCache, AddingEmptyStrings) +{ + cache.addStrings({}, mockStorageFetchDirectyId); +} +} // namespace