From a0c69c517c30bcff43ececc37817bd0b21e61ca7 Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Tue, 31 Jan 2017 12:18:08 +0100 Subject: [PATCH] Utils: Fix long small string We used only 6 bit to save the short size but for SmallString with a size over 64 it is not enough. So we have now to use a uint16 instead of a uint8 if the size if over 64. Change-Id: I53558e492b6cb40b739b23a8af83d192a2e11bd2 Reviewed-by: Tim Jenssen --- .../clangbackendipc/clangbackendipc_global.h | 2 +- src/libs/utils/smallstring.h | 9 +++- src/libs/utils/smallstringlayout.h | 53 ++++++++++++++----- src/libs/utils/smallstringliteral.h | 2 +- src/libs/utils/smallstringvector.h | 2 +- .../source/clangpchmanagerbackend_global.h | 2 +- tests/unit/unittest/smallstring-test.cpp | 10 ++++ 7 files changed, 61 insertions(+), 19 deletions(-) diff --git a/src/libs/clangbackendipc/clangbackendipc_global.h b/src/libs/clangbackendipc/clangbackendipc_global.h index 1da9a9f3230..5c12e159b82 100644 --- a/src/libs/clangbackendipc/clangbackendipc_global.h +++ b/src/libs/clangbackendipc/clangbackendipc_global.h @@ -55,7 +55,7 @@ namespace Utils { template class BasicSmallString; using SmallString = BasicSmallString<31>; -using PathString = BasicSmallString<191>; +using PathString = BasicSmallString<190>; } namespace ClangBackEnd { diff --git a/src/libs/utils/smallstring.h b/src/libs/utils/smallstring.h index fd5e208ecf1..4cc9638109a 100644 --- a/src/libs/utils/smallstring.h +++ b/src/libs/utils/smallstring.h @@ -65,6 +65,11 @@ public: using const_reverse_iterator = std::reverse_iterator; using size_type = std::size_t; + static_assert(Size < 64 + ? sizeof(Internal::StringDataLayout) == Size + 1 + : sizeof(Internal::StringDataLayout) == Size + 2, + "Size is wrong"); + BasicSmallString() noexcept : m_data(Internal::StringDataLayout()) { @@ -498,7 +503,7 @@ public: constexpr static size_type shortStringCapacity() noexcept { - return BasicSmallStringLiteral::shortStringCapacity(); + return Internal::StringDataLayout::shortStringCapacity(); } size_type optimalCapacity(const size_type size) @@ -897,6 +902,6 @@ std::vector clone(const std::vector &vector) } using SmallString = BasicSmallString<31>; -using PathString = BasicSmallString<191>; +using PathString = BasicSmallString<190>; } // namespace Utils diff --git a/src/libs/utils/smallstringlayout.h b/src/libs/utils/smallstringlayout.h index f58c5cacb3a..ca267b284e2 100644 --- a/src/libs/utils/smallstringlayout.h +++ b/src/libs/utils/smallstringlayout.h @@ -28,6 +28,7 @@ #include #include +#include #ifdef Q_CC_MSVC # define ALIGNAS_16 @@ -41,7 +42,19 @@ namespace Internal { using size_type = std::size_t; -template +template +struct block_type +{ + using type = uint8_t; +}; + +template<> +struct block_type { + using type = uint16_t; +}; + +template ::type> struct AllocatedLayout { struct Data { char *pointer; @@ -49,12 +62,13 @@ struct AllocatedLayout { size_type capacity; } data; char dummy[MaximumShortStringDataAreaSize - sizeof(Data)]; - std::uint8_t shortStringSize: 6; - std::uint8_t isReadOnlyReference : 1; - std::uint8_t isReference : 1; + BlockType shortStringSize : (sizeof(BlockType) * 8) - 2; + BlockType isReadOnlyReference : 1; + BlockType isReference : 1; }; -template +template ::type> struct ReferenceLayout { struct Data { const char *pointer; @@ -62,23 +76,28 @@ struct ReferenceLayout { size_type capacity; } data; char dummy[MaximumShortStringDataAreaSize - sizeof(Data)]; - std::uint8_t shortStringSize: 6; - std::uint8_t isReadOnlyReference : 1; - std::uint8_t isReference : 1; + BlockType shortStringSize : (sizeof(BlockType) * 8) - 2; + BlockType isReadOnlyReference : 1; + BlockType isReference : 1; }; -template +template ::type> struct ShortStringLayout { char string[MaximumShortStringDataAreaSize]; - std::uint8_t shortStringSize: 6; - std::uint8_t isReadOnlyReference : 1; - std::uint8_t isReference : 1; + BlockType shortStringSize : (sizeof(BlockType) * 8) - 2; + BlockType isReadOnlyReference : 1; + BlockType isReference : 1; }; template struct ALIGNAS_16 StringDataLayout { static_assert( MaximumShortStringDataAreaSize >= 15, "Size must be greater equal than 15 bytes!"); - static_assert(((MaximumShortStringDataAreaSize + 1) % 16) == 0, "Size + 1 must be dividable by 16!"); + 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 = default; constexpr StringDataLayout(const char *string, @@ -117,6 +136,14 @@ struct ALIGNAS_16 StringDataLayout { #endif } + constexpr static + size_type shortStringCapacity() noexcept + { + return MaximumShortStringDataAreaSize < 64 + ? MaximumShortStringDataAreaSize - 1 + : MaximumShortStringDataAreaSize - 2; + } + union { AllocatedLayout allocated; ReferenceLayout reference; diff --git a/src/libs/utils/smallstringliteral.h b/src/libs/utils/smallstringliteral.h index 4b7a6ba633c..3d5e1fef1e0 100644 --- a/src/libs/utils/smallstringliteral.h +++ b/src/libs/utils/smallstringliteral.h @@ -89,7 +89,7 @@ public: constexpr static size_type shortStringCapacity() noexcept { - return sizeof(Internal::ShortStringLayout) - 2; + return Internal::StringDataLayout::shortStringCapacity(); } bool isShortString() const noexcept diff --git a/src/libs/utils/smallstringvector.h b/src/libs/utils/smallstringvector.h index cc2c102a204..700a333ffb8 100644 --- a/src/libs/utils/smallstringvector.h +++ b/src/libs/utils/smallstringvector.h @@ -154,5 +154,5 @@ private: }; using SmallStringVector = BasicSmallStringVector<31>; -using PathStringVector = BasicSmallStringVector<191>; +using PathStringVector = BasicSmallStringVector<190>; } // namespace Utils; diff --git a/src/tools/clangpchmanagerbackend/source/clangpchmanagerbackend_global.h b/src/tools/clangpchmanagerbackend/source/clangpchmanagerbackend_global.h index b44fba700f0..9794f2bb60a 100644 --- a/src/tools/clangpchmanagerbackend/source/clangpchmanagerbackend_global.h +++ b/src/tools/clangpchmanagerbackend/source/clangpchmanagerbackend_global.h @@ -44,7 +44,7 @@ namespace Utils { template class BasicSmallString; using SmallString = BasicSmallString<31>; -using PathString = BasicSmallString<191>; +using PathString = BasicSmallString<190>; } namespace ClangBackEnd { diff --git a/tests/unit/unittest/smallstring-test.cpp b/tests/unit/unittest/smallstring-test.cpp index a0f2201bf7a..eb4ce8f7c3b 100644 --- a/tests/unit/unittest/smallstring-test.cpp +++ b/tests/unit/unittest/smallstring-test.cpp @@ -33,6 +33,7 @@ using namespace ::testing; +using Utils::PathString; using Utils::SmallString; using Utils::SmallStringLiteral; using Utils::SmallStringView; @@ -445,6 +446,15 @@ TEST(SmallString, SizeShortSmallString) ASSERT_THAT(size, 4); } +TEST(SmallString, SizeShortPathString) +{ + SmallString shortPath("very very very very very very very very very very very long path which fits in the short memory"); + + auto size = shortPath.size(); + + ASSERT_THAT(size, 95); +} + TEST(SmallString, SizeLongSmallString) { auto longText = SmallString::fromUtf8("very very very very very very very very very very very long string");