add configurable inplace storage

SergeyMakeev · Feb 3, 2024 · b0e561a · b0e561a
1 parent 2cd900f
commit b0e561a
Show file tree

Hide file tree

Showing 4 changed files with 122 additions and 30 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -18,6 +18,7 @@ set(TEST_SOURCES
   ExcaliburHashTest02.cpp
   ExcaliburHashTest03.cpp
   ExcaliburHashTest04.cpp
+  ExcaliburHashTest05.cpp
 )
 
 set (TEST_EXE_NAME ${PROJ_NAME})

diff --git a/ExcaliburHash/ExcaliburHash.h b/ExcaliburHash/ExcaliburHash.h
@@ -112,7 +112,7 @@ TODO: Design descisions/principles
 TODO: Memory layout
 
 */
-template <typename TKey, typename TValue, typename TKeyInfo = KeyInfo<TKey>> class HashTable
+template <typename TKey, typename TValue, typename TKeyInfo = KeyInfo<TKey>, unsigned kNumInlineItems = 1> class HashTable
 {
     struct has_values : std::bool_constant<!std::is_same<std::nullptr_t, typename std::remove_reference<TValue>::type>::value>
     {
@@ -241,7 +241,7 @@ template <typename TKey, typename TValue, typename TKeyInfo = KeyInfo<TKey>> cla
         if (!other.isUsingInlineStorage())
         {
             // if not using inline storage than it's a simple pointer swap
-            allocateInline(TKeyInfo::getEmpty());
+            constructInline(TKeyInfo::getEmpty());
             m_storage = other.m_storage;
             m_numBuckets = other.m_numBuckets;
             m_numElements = other.m_numElements;
@@ -252,23 +252,10 @@ template <typename TKey, typename TValue, typename TKeyInfo = KeyInfo<TKey>> cla
         else
         {
             // if using inline storage than let's move items from one inline storage into another
-            TItem* otherInlineItem = reinterpret_cast<TItem*>(&other.m_inlineStorage);
-            bool hasValidValue = otherInlineItem->isValid();
-            TItem* inlineItem = allocateInline(std::move(*otherInlineItem->key()));
+            TItem* otherInlineItems = reinterpret_cast<TItem*>(&other.m_inlineStorage);
+            TItem* inlineItems = moveInline(otherInlineItems);
 
-            if constexpr (has_values::value)
-            {
-                // move inline storage value (if any)
-                if (hasValidValue)
-                {
-                    TValue* value = inlineItem->value();
-                    TValue* otherValue = otherInlineItem->value();
-                    construct<TValue>(value, std::move(*otherValue));
-                    destruct(otherValue);
-                }
-            }
-
-            m_storage = inlineItem;
+            m_storage = inlineItems;
             m_numBuckets = other.m_numBuckets;
             m_numElements = other.m_numElements;
             // destruct(otherInlineItem);
@@ -310,11 +297,50 @@ template <typename TKey, typename TValue, typename TKeyInfo = KeyInfo<TKey>> cla
         return (inlineStorage == m_storage);
     }
 
-    template <class... Args> inline TItem* allocateInline(Args&&... args)
+    template <class... Args> inline TItem* constructInline(Args&&... args)
+    {
+        TItem* inlineItems = reinterpret_cast<TItem*>(&m_inlineStorage);
+        for (unsigned i = 0; i < kNumInlineItems; i++)
+        {
+            construct<TItem>((inlineItems + i), std::forward<Args>(args)...);
+        }
+        return inlineItems;
+    }
+
+    inline TItem* moveInline(TItem* from)
     {
-        TItem* inlineItem = reinterpret_cast<TItem*>(&m_inlineStorage);
-        construct<TItem>(inlineItem, std::forward<Args>(args)...);
-        return inlineItem;
+        TItem* inlineItems = reinterpret_cast<TItem*>(&m_inlineStorage);
+
+        if constexpr (has_values::value)
+        {
+            // move all keys and valid values
+            for (unsigned i = 0; i < kNumInlineItems; i++)
+            {
+                TItem* inlineItem = (inlineItems + i);
+                TItem& otherInlineItem = from[i];
+                const bool hasValidValue = otherInlineItem.isValid();
+                construct<TItem>((inlineItems + i), std::move(*otherInlineItem.key()));
+
+                // move inline storage value (if any)
+                if (hasValidValue)
+                {
+                    TValue* value = inlineItem->value();
+                    TValue* otherValue = otherInlineItem.value();
+                    construct<TValue>(value, std::move(*otherValue));
+                    destruct(otherValue);
+                }
+            }
+        }
+        else
+        {
+            // move only keys 
+            for (unsigned i = 0; i < kNumInlineItems; i++)
+            {
+                construct<TItem>((inlineItems + i), std::move(*from[i].key()));
+            }
+        }
+
+        return inlineItems;
     }
 
     inline uint32_t create(uint32_t numBuckets)
@@ -476,7 +502,7 @@ template <typename TKey, typename TValue, typename TKeyInfo = KeyInfo<TKey>> cla
       protected:
         const HashTable* m_ht;
         TItem* m_item;
-        friend class HashTable<TKey, TValue, TKeyInfo>;
+        friend class HashTable<TKey, TValue, TKeyInfo, kNumInlineItems>;
     };
 
     class IteratorK : public IteratorBase
@@ -592,10 +618,10 @@ template <typename TKey, typename TValue, typename TKeyInfo = KeyInfo<TKey>> cla
 
     HashTable() noexcept
         //: m_storage(nullptr)
-        : m_numBuckets(1)
+        : m_numBuckets(kNumInlineItems)
         , m_numElements(0)
     {
-        m_storage = allocateInline(TKeyInfo::getEmpty());
+        m_storage = constructInline(TKeyInfo::getEmpty());
     }
 
     ~HashTable()
@@ -933,7 +959,7 @@ template <typename TKey, typename TValue, typename TKeyInfo = KeyInfo<TKey>> cla
     HashTable(const HashTable& other)
     {
         EXLBR_ASSERT(&other != this);
-        m_storage = allocateInline(TKeyInfo::getEmpty());
+        m_storage = constructInline(TKeyInfo::getEmpty());
         create(other.m_numBuckets);
         copyFrom(other);
     }
@@ -946,7 +972,7 @@ template <typename TKey, typename TValue, typename TKeyInfo = KeyInfo<TKey>> cla
             return *this;
         }
         destroyAndFreeMemory();
-        m_storage = allocateInline(TKeyInfo::getEmpty());
+        m_storage = constructInline(TKeyInfo::getEmpty());
         create(other.m_numBuckets);
         copyFrom(other);
         return *this;
@@ -982,8 +1008,9 @@ template <typename TKey, typename TValue, typename TKeyInfo = KeyInfo<TKey>> cla
 
     // We need this inline storage to keep `m_storage` not null all the time.
     // This will save us from `empty()` check inside `find()` function implementation
-    typename std::aligned_storage<sizeof(TItem), alignof(TItem)>::type m_inlineStorage;
-    static_assert(sizeof(m_inlineStorage) == sizeof(TItem), "Incorrect sizeof");
+    static_assert(kNumInlineItems != 0, "Num inline items can't be zero!");
+    typename std::aligned_storage<sizeof(TItem) * kNumInlineItems, alignof(TItem)>::type m_inlineStorage;
+    static_assert(sizeof(m_inlineStorage) == (sizeof(TItem) * kNumInlineItems), "Incorrect sizeof");
 };
 
 } // namespace Excalibur
diff --git a/ExcaliburHashTest02.cpp b/ExcaliburHashTest02.cpp
@@ -60,7 +60,7 @@ TEST(SmFlatHashMap, CtorDtorCallCount)
 
     {
         // empty hash table
-        Excalibur::HashTable<ComplexStruct, int> ht;
+        Excalibur::HashTable<ComplexStruct, int, Excalibur::KeyInfo<ComplexStruct>, 4> ht;
         EXPECT_TRUE(ht.empty());
         EXPECT_EQ(ht.size(), 0u);
         EXPECT_GE(ht.capacity(), 0u);

diff --git a/ExcaliburHashTest05.cpp b/ExcaliburHashTest05.cpp
@@ -0,0 +1,64 @@
+#include "ExcaliburHash.h"
+#include "gtest/gtest.h"
+#include <array>
+#include <cstring>
+
+namespace Excalibur
+{
+template <> struct KeyInfo<std::string>
+{
+    static inline bool isValid(const std::string& key) noexcept { return !key.empty() && key.data()[0] != char(1); }
+    static inline std::string getTombstone() noexcept
+    {
+        // and let's hope that small string optimization will do the job
+        return std::string(1, char(1));
+    }
+    static inline std::string getEmpty() noexcept { return std::string(); }
+    static inline uint64_t hash(const std::string& key) noexcept { return std::hash<std::string>{}(key); }
+    static inline bool isEqual(const std::string& lhs, const std::string& rhs) noexcept { return lhs == rhs; }
+};
+} // namespace Excalibur
+
+TEST(SmFlatHashMap, InlineStorageTest01)
+{
+    // create hash map and insert one element
+    Excalibur::HashTable<std::string, std::string, Excalibur::KeyInfo<std::string>, 4> ht;
+
+    EXPECT_GE(ht.capacity(), uint32_t(4));
+
+    auto it1 = ht.emplace(std::string("hello1"), std::string("world1"));
+    EXPECT_TRUE(it1.second);
+    auto it2 = ht.emplace(std::string("hello2"), std::string("world2"));
+    EXPECT_TRUE(it2.second);
+
+    EXPECT_EQ(ht.size(), uint32_t(2));
+
+    {
+        auto _it1 = ht.find("hello1");
+        ASSERT_NE(_it1, ht.end());
+        const std::string& val1 = _it1->second;
+        ASSERT_EQ(val1, "world1");
+
+        auto _it2 = ht.find("hello2");
+        ASSERT_NE(_it2, ht.end());
+        const std::string& val2 = _it2->second;
+        ASSERT_EQ(val2, "world2");
+    }
+
+    for (int i = 0; i < 1000; i++)
+    {
+        ht.emplace(std::to_string(i), "tmp");
+    }
+
+    {
+        auto _it1 = ht.find("hello1");
+        ASSERT_NE(_it1, ht.end());
+        const std::string& val1 = _it1->second;
+        ASSERT_EQ(val1, "world1");
+
+        auto _it2 = ht.find("hello2");
+        ASSERT_NE(_it2, ht.end());
+        const std::string& val2 = _it2->second;
+        ASSERT_EQ(val2, "world2");
+    }
+}