From be939a80db584aecf94fc83305cd1529f4ddb51d Mon Sep 17 00:00:00 2001
From: Marco Bubke <marco.bubke@qt.io>
Date: Thu, 5 Oct 2017 17:36:44 +0200
Subject: Clang: Improve locking of string cache

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>
---
 tests/unit/unittest/stringcache-test.cpp | 86 +++++++++++++++++++++++++++-----
 1 file changed, 74 insertions(+), 12 deletions(-)

(limited to 'tests/unit/unittest/stringcache-test.cpp')

diff --git a/tests/unit/unittest/stringcache-test.cpp b/tests/unit/unittest/stringcache-test.cpp
index 1a20e473de..8465321c34 100644
--- a/tests/unit/unittest/stringcache-test.cpp
+++ b/tests/unit/unittest/stringcache-test.cpp
@@ -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, StringIdsIsLocked)
+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_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)
-- 
cgit v1.2.1