feedc0de/qt-creator
1
0
Fork 0
forked from qt-creator/qt-creator
Code Pull Requests Activity

Clang: Improve locking of string cache

Browse Source 
The string cache is only very seldom written but very often read. To
improve thread scaling it is faster to lock it only for write operations.
So we use a shared mutex which is locked in shared mode for read
operations and locked exclusively for write operations.

Change-Id: I2cc742d1b9cc15c162be40ab339fa8310640bc44
Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>
This commit is contained in:
Marco Bubke
2017-10-05 17:36:44 +02:00
committed by Tim Jenssen
parent d2e15e5f1e
commit be939a80db
4 changed files with 122 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/libs/clangsupport/filepathcache.h
View File

@@ -67,12 +67,12 @@ class FilePathCache final : private FilePathCacheBase
{
using DirectoryPathCache = StringCache<Utils::PathString,
int,
std::mutex,
std::shared_timed_mutex,
decltype(&Utils::reverseCompare),
Utils::reverseCompare>;
using FileNameCache = StringCache<Utils::SmallString,
int,
std::mutex,
std::shared_timed_mutex,
decltype(&Utils::compare),
Utils::compare>;
public:

77
src/libs/clangsupport/stringcache.h
View File

@@ -33,7 +33,7 @@
#include <utils/smallstringfwd.h>
#include <algorithm>
#include <mutex>
#include <shared_mutex>
#include <vector>
namespace ClangBackEnd {
@@ -54,6 +54,8 @@ public:
NonLockingMutex& operator=(const NonLockingMutex&) = delete;
void lock() {}
void unlock() {}
void lock_shared() {}
void unlock_shared() {}
};
template <typename StringType, typename IndexType>
@@ -122,23 +124,34 @@ public:
IndexType stringId(Utils::SmallStringView stringView)
{
std::lock_guard<Mutex> lock(m_mutex);
std::shared_lock<Mutex> sharedLock(m_mutex);
Found found = find(stringView);
return ungardedStringId(stringView);
if (found.wasFound)
return found.iterator->id;
sharedLock.unlock();
std::lock_guard<Mutex> exclusiveLock(m_mutex);
found = find(stringView);
if (!found.wasFound) {
IndexType index = insertString(found.iterator, stringView, IndexType(m_indices.size()));
found.iterator = m_strings.begin() + index;;
}
return found.iterator->id;
}
template <typename Container>
std::vector<IndexType> stringIds(const Container &strings)
{
std::lock_guard<Mutex> lock(m_mutex);
std::vector<IndexType> ids;
ids.reserve(strings.size());
std::transform(strings.begin(),
strings.end(),
std::back_inserter(ids),
[&] (const auto &string) { return this->ungardedStringId(string); });
[&] (const auto &string) { return this->stringId(string); });
return ids;
}
@@ -150,7 +163,7 @@ public:
StringType string(IndexType id) const
{
std::lock_guard<Mutex> lock(m_mutex);
std::shared_lock<Mutex> sharedLock(m_mutex);
return m_strings.at(m_indices.at(id)).string;
}
@@ -158,28 +171,25 @@ public:
template<typename Function>
StringType string(IndexType id, Function storageFunction)
{
std::lock_guard<Mutex> lock(m_mutex);
std::shared_lock<Mutex> sharedLock(m_mutex);
if (IndexType(m_indices.size()) > id && m_indices.at(id) >= 0)
return m_strings.at(m_indices.at(id)).string;
sharedLock.unlock();
std::lock_guard<Mutex> exclusiveLock(m_mutex);
IndexType index;
if (IndexType(m_indices.size()) <= id) {
StringType string{storageFunction(id)};
index = insertString(find(string).iterator, string, id);
} else {
index = m_indices.at(id);
StringType string{storageFunction(id)};
index = insertString(find(string).iterator, string, id);
if (index < 0) {
StringType string{storageFunction(id)};
index = insertString(find(string).iterator, string, id);
}
}
return m_strings.at(index).string;
return m_strings[index].string;
}
std::vector<StringType> strings(const std::vector<IndexType> &ids) const
{
std::lock_guard<Mutex> lock(m_mutex);
std::shared_lock<Mutex> sharedLock(m_mutex);
std::vector<StringType> strings;
strings.reserve(ids.size());
@@ -200,12 +210,21 @@ public:
template<typename Function>
IndexType stringId(Utils::SmallStringView stringView, Function storageFunction)
{
std::lock_guard<Mutex> lock(m_mutex);
std::shared_lock<Mutex> sharedLock(m_mutex);
Found found = find(stringView);
if (!found.wasFound)
insertString(found.iterator, stringView, storageFunction(stringView));
if (found.wasFound)
return found.iterator->id;
sharedLock.unlock();
std::lock_guard<Mutex> exclusiveLock(m_mutex);
found = find(stringView);
if (!found.wasFound) {
IndexType index = insertString(found.iterator, stringView, storageFunction(stringView));
found.iterator = m_strings.begin() + index;
}
return found.iterator->id;
}
@@ -216,16 +235,6 @@ public:
}
private:
IndexType ungardedStringId(Utils::SmallStringView stringView)
{
Found found = find(stringView);
if (!found.wasFound)
insertString(found.iterator, stringView, IndexType(m_indices.size()));
return found.iterator->id;
}
Found find(Utils::SmallStringView stringView)
{
return findInSorted(m_strings.cbegin(), m_strings.cend(), stringView, compare);

2
tests/unit/unittest/mockmutex.h
View File

@@ -34,4 +34,6 @@ public:
MOCK_METHOD0(lock, void ());
MOCK_METHOD0(unlock, void ());
MOCK_METHOD0(lock_shared, void ());
MOCK_METHOD0(unlock_shared, void ());
};

88
tests/unit/unittest/stringcache-test.cpp
View File

@@ -329,48 +329,110 @@ TEST_F(StringCache, FindInSortedFifeReverse)
ASSERT_TRUE(found.wasFound);
}
TEST_F(StringCache, StringIdIsLocked)
TEST_F(StringCache, StringIdIsReadAndWriteLockedForUnknownEntry)
{
InSequence s;
EXPECT_CALL(mockMutex, lock_shared());
EXPECT_CALL(mockMutex, unlock_shared());
EXPECT_CALL(mockMutex, lock());
EXPECT_CALL(mockMutex, unlock());
cache.stringId("foo");
}
TEST_F(StringCache, StringIdWithStorageFunctionIsReadAndWriteLockedForUnknownEntry)
{
InSequence s;
EXPECT_CALL(mockMutex, lock_shared());
EXPECT_CALL(mockMutex, unlock_shared());
EXPECT_CALL(mockMutex, lock());
EXPECT_CALL(mockStorage, fetchDirectoryId(Eq("foo")));
EXPECT_CALL(mockMutex, unlock());
cache.stringId("foo", mockStorageFetchDirectyId);
}
TEST_F(StringCache, StringIdWithStorageFunctionIsReadLockedForKnownEntry)
{
InSequence s;
cache.stringId("foo", mockStorageFetchDirectyId);
EXPECT_CALL(mockMutex, lock_shared());
EXPECT_CALL(mockMutex, unlock_shared());
EXPECT_CALL(mockMutex, lock()).Times(0);
EXPECT_CALL(mockStorage, fetchDirectoryId(Eq("foo"))).Times(0);
EXPECT_CALL(mockMutex, unlock()).Times(0);
cache.stringId("foo", mockStorageFetchDirectyId);
}
TEST_F(StringCache, StringIdIsReadLockedForKnownEntry)
{
cache.stringId("foo");
EXPECT_CALL(mockMutex, lock_shared());
EXPECT_CALL(mockMutex, unlock_shared());
EXPECT_CALL(mockMutex, lock()).Times(0);
EXPECT_CALL(mockMutex, unlock()).Times(0);
cache.stringId("foo");
}
TEST_F(StringCache, StringIdsIsLocked)
{
EXPECT_CALL(mockMutex, lock());
EXPECT_CALL(mockMutex, unlock());
EXPECT_CALL(mockMutex, lock_shared());
EXPECT_CALL(mockMutex, unlock_shared());
cache.stringIds({"foo"});
}
TEST_F(StringCache, StringIsLocked)
{
cache.stringId("foo");
auto id = cache.stringId("foo");
EXPECT_CALL(mockMutex, lock());
EXPECT_CALL(mockMutex, unlock());
EXPECT_CALL(mockMutex, lock_shared());
EXPECT_CALL(mockMutex, unlock_shared());
cache.string(0);
cache.string(id);
}
TEST_F(StringCache, StringsIsLocked)
{
cache.stringId("foo");
auto ids = cache.stringIds({"foo", "bar"});
EXPECT_CALL(mockMutex, lock());
EXPECT_CALL(mockMutex, unlock());
EXPECT_CALL(mockMutex, lock_shared());
EXPECT_CALL(mockMutex, unlock_shared());
cache.strings({0});
cache.strings(ids);
}
TEST_F(StringCache, StringIdWithStorageFunctionIsLocked)
TEST_F(StringCache, StringWithStorageFunctionIsReadAndWriteLockedForUnknownId)
{
InSequence s;
EXPECT_CALL(mockMutex, lock_shared());
EXPECT_CALL(mockMutex, unlock_shared());
EXPECT_CALL(mockMutex, lock());
EXPECT_CALL(mockStorage, fetchDirectoryPath(Eq(41)));
EXPECT_CALL(mockMutex, unlock());
cache.stringId("foo", mockStorageFetchDirectyId);
cache.string(41, mockStorageFetchDirectyPath);
}
TEST_F(StringCache, StringWithStorageFunctionIsReadLockedForKnownId)
{
InSequence s;
cache.string(41, mockStorageFetchDirectyPath);
EXPECT_CALL(mockMutex, lock_shared());
EXPECT_CALL(mockMutex, unlock_shared());
EXPECT_CALL(mockMutex, lock()).Times(0);
EXPECT_CALL(mockStorage, fetchDirectoryPath(Eq(41))).Times(0);
EXPECT_CALL(mockMutex, unlock()).Times(0);
cache.string(41, mockStorageFetchDirectyPath);
}
TEST_F(StringCache, StringIdWithStorageFunctionWhichHasNoEntryIsCallingStorageFunction)