Padding chunks in Arena in attempt to improve performance [#CLICKHOUSE-2]

dbms/src/Common/Arena.h

@@ -6,6 +6,7 @@
 #include <boost/noncopyable.hpp>
 #include <common/likely.h>
 #include <Core/Defines.h>
+#include <Common/memcpySmall.h>
 #include <Common/ProfileEvents.h>
 #include <Common/Allocator.h>
 
@@ -31,12 +32,15 @@ namespace DB
 class Arena : private boost::noncopyable
 {
 private:
+    /// Padding allows to use 'memcpySmallAllowReadWriteOverflow15' instead of 'memcpy'.
+    static constexpr size_t pad_right = 15;
+
     /// Contiguous chunk of memory and pointer to free space inside it. Member of single-linked list.
     struct Chunk : private Allocator<false>    /// empty base optimization
     {
         char * begin;
         char * pos;
-        char * end;
+        char * end; /// does not include padding.
 
         Chunk * prev;
 
@@ -47,7 +51,7 @@ private:
 
             begin = reinterpret_cast<char *>(Allocator::alloc(size_));
             pos = begin;
-            end = begin + size_;
+            end = begin + size_ - pad_right;
             prev = prev_;
         }
 
@@ -59,7 +63,7 @@ private:
                 delete prev;
         }
 
-        size_t size() const { return end - begin; }
+        size_t size() const { return end + pad_right - begin; }
         size_t remaining() const { return end - pos; }
     };
 
@@ -95,7 +99,7 @@ private:
     /// Add next contiguous chunk of memory with size not less than specified.
     void NO_INLINE addChunk(size_t min_size)
     {
-        head = new Chunk(nextSize(min_size), head);
+        head = new Chunk(nextSize(min_size + pad_right), head);
         size_in_bytes += head->size();
     }
 
@@ -216,7 +220,7 @@ public:
     {
         char * res = alloc(new_size);
         if (old_data)
-            memcpy(res, old_data, old_size);
+            memcpySmallAllowReadWriteOverflow15(res, old_data, old_size);
         return res;
     }
 
@@ -224,7 +228,7 @@ public:
     {
         char * res = alignedAlloc(new_size, alignment);
         if (old_data)
-            memcpy(res, old_data, old_size);
+            memcpySmallAllowReadWriteOverflow15(res, old_data, old_size);
         return res;
     }
 
@@ -232,14 +236,14 @@ public:
     const char * insert(const char * data, size_t size)
     {
         char * res = alloc(size);
-        memcpy(res, data, size);
+        memcpySmallAllowReadWriteOverflow15(res, data, size);
         return res;
     }
 
     const char * alignedInsert(const char * data, size_t size, size_t alignment)
     {
         char * res = alignedAlloc(size, alignment);
-        memcpy(res, data, size);
+        memcpySmallAllowReadWriteOverflow15(res, data, size);
         return res;
     }
 

dbms/src/Interpreters/AggregationCommon.h

@@ -6,6 +6,7 @@
 #include <Common/Arena.h>
 #include <Common/UInt128.h>
 #include <Common/HashTable/Hash.h>
+#include <Common/memcpySmall.h>
 #include <Core/Defines.h>
 #include <common/StringRef.h>
 #include <Columns/IColumn.h>
@@ -215,33 +216,13 @@ static inline StringRef * ALWAYS_INLINE placeKeysInPool(
     for (size_t j = 0; j < keys_size; ++j)
     {
         char * place = pool.alloc(keys[j].size);
-        memcpy(place, keys[j].data, keys[j].size);        /// TODO padding in Arena and memcpySmall
+        memcpySmallAllowReadWriteOverflow15(place, keys[j].data, keys[j].size);
         keys[j].data = place;
     }
 
     /// Place the StringRefs on the newly copied keys in the pool.
-    char * res = pool.alloc(keys_size * sizeof(StringRef));
+    char * res = pool.alignedAlloc(keys_size * sizeof(StringRef), alignof(StringRef));
-    memcpy(res, keys.data(), keys_size * sizeof(StringRef));
+    memcpySmallAllowReadWriteOverflow15(res, keys.data(), keys_size * sizeof(StringRef));
-
-    return reinterpret_cast<StringRef *>(res);
-}
-
-
-/// Copy keys to the pool. Then put into pool StringRefs to them and return the pointer to the first.
-static inline StringRef * ALWAYS_INLINE extractKeysAndPlaceInPool(
-    size_t i, size_t keys_size, const ColumnRawPtrs & key_columns, StringRefs & keys, Arena & pool)
-{
-    for (size_t j = 0; j < keys_size; ++j)
-    {
-        keys[j] = key_columns[j]->getDataAtWithTerminatingZero(i);
-        char * place = pool.alloc(keys[j].size);
-        memcpy(place, keys[j].data, keys[j].size);
-        keys[j].data = place;
-    }
-
-    /// Place the StringRefs on the newly copied keys in the pool.
-    char * res = pool.alloc(keys_size * sizeof(StringRef));
-    memcpy(res, keys.data(), keys_size * sizeof(StringRef));
 
     return reinterpret_cast<StringRef *>(res);
 }
@@ -251,15 +232,18 @@ static inline StringRef * ALWAYS_INLINE extractKeysAndPlaceInPool(
 /// Subsequently append StringRef objects referring to each key.
 ///
 /// [key1][key2]...[keyN][ref1][ref2]...[refN]
-///   ^     ^        :     |     |
+///   ^     ^        :     |      |
-///   +-----|--------:-----+     |
+///   +-----|--------:-----+      |
-///   :     +--------:-----------+
+///   :     +--------:------------+
 ///   :              :
 ///   <-------------->
 ///        (1)
 ///
 /// Return a StringRef object, referring to the area (1) of the memory
 /// chunk that contains the keys. In other words, we ignore their StringRefs.
+///
+/// NOTE Storing sizes instead of StringRefs may be beneficial.
+/// NOTE This method has great potential for specialization with runtime code generation.
 inline StringRef ALWAYS_INLINE extractKeysAndPlaceInPoolContiguous(
     size_t i, size_t keys_size, const ColumnRawPtrs & key_columns, StringRefs & keys, Arena & pool)
 {
@@ -275,13 +259,13 @@ inline StringRef ALWAYS_INLINE extractKeysAndPlaceInPoolContiguous(
 
     for (size_t j = 0; j < keys_size; ++j)
     {
-        memcpy(place, keys[j].data, keys[j].size);
+        memcpySmallAllowReadWriteOverflow15(place, keys[j].data, keys[j].size);
         keys[j].data = place;
         place += keys[j].size;
     }
 
     /// Place the StringRefs on the newly copied keys in the pool.
-    memcpy(place, keys.data(), keys_size * sizeof(StringRef));
+    memcpySmallAllowReadWriteOverflow15(place, keys.data(), keys_size * sizeof(StringRef));
 
     return {res, sum_keys_size};
 }

dbms/src/Interpreters/Aggregator.h

@@ -547,7 +547,8 @@ private:
     static void insertKeyIntoColumnsImpl(const typename Data::value_type & value, MutableColumns & key_columns, size_t keys_size, const Sizes &)
     {
         /// See function extractKeysAndPlaceInPoolContiguous.
-        const StringRef * key_refs = reinterpret_cast<const StringRef *>(value.first.data + value.first.size);
+        using UnalignedStringRef = StringRef __attribute__((__aligned__(1)));
+        const UnalignedStringRef * key_refs = reinterpret_cast<const UnalignedStringRef *>(value.first.data + value.first.size);
 
         if (unlikely(0 == value.first.size))
         {