From fa32b652c81f40187c930a7dbb0b9885a288c580 Mon Sep 17 00:00:00 2001
From: Marco Bubke <marco.bubke@qt.io>
Date: Sat, 30 Jul 2022 19:08:41 +0200
Subject: [PATCH] Utils: Optimze SmallString copies for larger sizes

So far the whole class was always copied. For a large SmallString
classes this were often many zeros if it was empty. Now only the non
zero area is copied in that case.

Change-Id: Ic0ea2d6f763513858a993bba3e829c9fc947e1a6
Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
---
 src/libs/utils/smallstringlayout.h       | 103 ++++++++++++++++++-----
 tests/unit/unittest/smallstring-test.cpp |  82 ++++++++++++++++++
 2 files changed, 162 insertions(+), 23 deletions(-)

diff --git a/src/libs/utils/smallstringlayout.h b/src/libs/utils/smallstringlayout.h
index dd9cf7a817a..7d6190528ef 100644
--- a/src/libs/utils/smallstringlayout.h
+++ b/src/libs/utils/smallstringlayout.h
@@ -28,6 +28,7 @@
 #include <QtGlobal>
 
 #include <cstdint>
+#include <cstring>
 #include <memory>
 #include <type_traits>
 
@@ -95,8 +96,8 @@ private:
     ControlType m_isReference : 1;
 };
 
-template <uint MaximumShortStringDataAreaSize>
-struct AllocatedLayout
+template<uint MaximumShortStringDataAreaSize>
+struct alignas(16) AllocatedLayout
 {
     struct Data
     {
@@ -109,8 +110,8 @@ struct AllocatedLayout
     Data data;
 };
 
-template <uint MaximumShortStringDataAreaSize>
-struct ReferenceLayout
+template<uint MaximumShortStringDataAreaSize>
+struct alignas(16) ReferenceLayout
 {
     constexpr ReferenceLayout() noexcept = default;
     constexpr ReferenceLayout(const char *stringPointer,
@@ -131,10 +132,9 @@ struct ReferenceLayout
     Data data;
 };
 
-template <uint MaximumShortStringDataAreaSize>
-struct ShortStringLayout
+template<uint MaximumShortStringDataAreaSize>
+struct alignas(16) ShortStringLayout
 {
-    constexpr ShortStringLayout() noexcept = default;
     constexpr ShortStringLayout(
         typename ControlBlock<MaximumShortStringDataAreaSize>::SizeType shortStringSize) noexcept
         : control(shortStringSize, false, false)
@@ -144,22 +144,19 @@ struct ShortStringLayout
     char string[MaximumShortStringDataAreaSize];
 };
 
-template <uint MaximumShortStringDataAreaSize>
-struct StringDataLayout {
+template<uint MaximumShortStringDataAreaSize, typename = void>
+struct StringDataLayout
+{
     static_assert(MaximumShortStringDataAreaSize >= 15, "Size must be greater equal than 15 bytes!");
-    static_assert(MaximumShortStringDataAreaSize < 64
-                ? ((MaximumShortStringDataAreaSize + 1) % 16) == 0
-                : ((MaximumShortStringDataAreaSize + 2) % 16) == 0,
-                  "Size + 1 must be dividable by 16 if under 64 and Size + 2 must be dividable by 16 if over 64!");
+    static_assert(MaximumShortStringDataAreaSize < 32, "Size must be less than 32 bytes!");
+    static_assert(((MaximumShortStringDataAreaSize + 1) % 16) == 0,
+                  "Size + 1 must be dividable by 16 if under 64 and Size + 2 must be dividable by "
+                  "16 if over 64!");
 
-    StringDataLayout() noexcept
-    {
-        reset();
-    }
+    StringDataLayout() noexcept { reset(); }
 
-    constexpr StringDataLayout(const char *string,
-                               size_type size) noexcept
-       : reference(string, size, 0)
+    constexpr StringDataLayout(const char *string, size_type size) noexcept
+        : reference(string, size, 0)
     {}
 
     template<size_type Size>
@@ -174,13 +171,73 @@ struct StringDataLayout {
         }
     }
 
-    constexpr static
-    size_type shortStringCapacity() noexcept
+    constexpr static size_type shortStringCapacity() noexcept
     {
         return MaximumShortStringDataAreaSize - 1;
     }
 
-    void reset()
+    constexpr void reset()
+    {
+        shortString.control = ControlBlock<MaximumShortStringDataAreaSize>();
+        shortString.string[0] = '\0';
+    }
+
+    union {
+        AllocatedLayout<MaximumShortStringDataAreaSize> allocated;
+        ReferenceLayout<MaximumShortStringDataAreaSize> reference;
+        ShortStringLayout<MaximumShortStringDataAreaSize> shortString;
+    };
+};
+
+template<uint MaximumShortStringDataAreaSize>
+struct StringDataLayout<MaximumShortStringDataAreaSize,
+                        std::enable_if_t<MaximumShortStringDataAreaSize >= 32>>
+{
+    static_assert(MaximumShortStringDataAreaSize > 31, "Size must be greater than 31 bytes!");
+    static_assert(MaximumShortStringDataAreaSize < 64
+                      ? ((MaximumShortStringDataAreaSize + 1) % 16) == 0
+                      : ((MaximumShortStringDataAreaSize + 2) % 16) == 0,
+                  "Size + 1 must be dividable by 16 if under 64 and Size + 2 must be dividable by "
+                  "16 if over 64!");
+
+    StringDataLayout() noexcept { reset(); }
+
+    constexpr StringDataLayout(const char *string, size_type size) noexcept
+        : reference(string, size, 0)
+    {}
+
+    template<size_type Size>
+    constexpr StringDataLayout(const char (&string)[Size]) noexcept
+    {
+        if constexpr (Size <= MaximumShortStringDataAreaSize) {
+            shortString = {Size - 1};
+            for (size_type i = 0; i < Size; ++i)
+                shortString.string[i] = string[i];
+        } else {
+            reference = {string, Size - 1, 0};
+        }
+    }
+
+    StringDataLayout(const StringDataLayout &other) noexcept { *this = other; }
+
+    StringDataLayout &operator=(const StringDataLayout &other) noexcept
+    {
+        constexpr auto controlBlockSize = sizeof(ControlBlock<MaximumShortStringDataAreaSize>);
+        auto shortStringLayoutSize = other.shortString.control.stringSize() + controlBlockSize;
+        constexpr auto referenceLayoutSize = sizeof(ReferenceLayout<MaximumShortStringDataAreaSize>);
+        std::memcpy(&shortString,
+                    &other.shortString,
+                    std::max(shortStringLayoutSize, referenceLayoutSize));
+
+        return *this;
+    }
+
+    constexpr static size_type shortStringCapacity() noexcept
+    {
+        return MaximumShortStringDataAreaSize - 1;
+    }
+
+    constexpr void reset()
     {
         shortString.control = ControlBlock<MaximumShortStringDataAreaSize>();
         shortString.string[0] = '\0';
diff --git a/tests/unit/unittest/smallstring-test.cpp b/tests/unit/unittest/smallstring-test.cpp
index a45c36918f6..0972a350b2d 100644
--- a/tests/unit/unittest/smallstring-test.cpp
+++ b/tests/unit/unittest/smallstring-test.cpp
@@ -38,6 +38,14 @@ using Utils::SmallString;
 using Utils::SmallStringLiteral;
 using Utils::SmallStringView;
 
+static_assert(32 == sizeof(Utils::BasicSmallString<31>));
+static_assert(64 == sizeof(Utils::BasicSmallString<63>));
+static_assert(192 == sizeof(Utils::BasicSmallString<190>));
+
+static_assert(16 == alignof(Utils::BasicSmallString<31>));
+static_assert(16 == alignof(Utils::BasicSmallString<63>));
+static_assert(16 == alignof(Utils::BasicSmallString<190>));
+
 TEST(SmallString, BasicStringEqual)
 {
     ASSERT_THAT(SmallString("text"), Eq(SmallString("text")));
@@ -1524,6 +1532,80 @@ TEST(SmallString, LongSmallStringMoveConstuctor)
     ASSERT_THAT(copy, SmallString("this is a very very very very long text"));
 }
 
+TEST(SmallString, ShortPathStringMoveConstuctor)
+{
+    PathString text("text");
+
+    auto copy = std::move(text);
+
+    ASSERT_TRUE(text.isEmpty());
+    ASSERT_THAT(copy, SmallString("text"));
+}
+
+TEST(SmallString, LongPathStringMoveConstuctor)
+{
+    PathString text(
+        "this is a very very very very very very very very very very very very very very very very "
+        "very very very very very very very very very very very very very very very very very very "
+        "very very very very very very very very very very very very very very long text");
+
+    auto copy = std::move(text);
+
+    ASSERT_TRUE(text.isEmpty());
+    ASSERT_THAT(
+        copy,
+        PathString(
+            "this is a very very very very very very very very very very very very very very very "
+            "very very very very very very very very very very very very very very very very very "
+            "very very very very very very very very very very very very very very very very long "
+            "text"));
+}
+
+TEST(SmallString, ShortSmallStringMoveConstuctorToSelf)
+{
+    SmallString text("text");
+
+    text = std::move(text);
+
+    ASSERT_THAT(text, SmallString("text"));
+}
+
+TEST(SmallString, LongSmallStringMoveConstuctorToSelf)
+{
+    SmallString text("this is a very very very very long text");
+
+    text = std::move(text);
+
+    ASSERT_THAT(text, SmallString("this is a very very very very long text"));
+}
+
+TEST(SmallString, ShortPathStringMoveConstuctorToSelf)
+{
+    PathString text("text");
+
+    text = std::move(text);
+
+    ASSERT_THAT(text, SmallString("text"));
+}
+
+TEST(SmallString, LongPathStringMoveConstuctorToSelf)
+{
+    PathString text(
+        "this is a very very very very very very very very very very very very very very very very "
+        "very very very very very very very very very very very very very very very very very very "
+        "very very very very very very very very very very very very very very long text");
+
+    text = std::move(text);
+
+    ASSERT_THAT(
+        text,
+        PathString(
+            "this is a very very very very very very very very very very very very very very very "
+            "very very very very very very very very very very very very very very very very very "
+            "very very very very very very very very very very very very very very very very long "
+            "text"));
+}
+
 TEST(SmallString, ShortSmallStringCopyAssignment)
 {
     SmallString text("text");