RCORE-2050 New Array Header support for compression (#7521)

* code review

---------

Co-authored-by: Finn Schiermer Andersen <[email protected]>
Co-authored-by: Jørgen Edelbo <[email protected]>

Author: 3 people

src/realm/alloc.cpp

@@ -113,7 +113,8 @@ Allocator& Allocator::get_default() noexcept
 // * adding a cross-over mapping. (if the array crosses a mapping boundary)
 // * using an already established cross-over mapping. (ditto)
 // this can proceed concurrently with other calls to translate()
-char* Allocator::translate_less_critical(RefTranslation* ref_translation_ptr, ref_type ref) const noexcept
+char* Allocator::translate_less_critical(RefTranslation* ref_translation_ptr, ref_type ref,
+                                         bool known_in_slab) const noexcept
 {
     size_t idx = get_section_index(ref);
     RefTranslation& txl = ref_translation_ptr[idx];
@@ -122,7 +123,9 @@ char* Allocator::translate_less_critical(RefTranslation* ref_translation_ptr, re
 #if REALM_ENABLE_ENCRYPTION
     realm::util::encryption_read_barrier(addr, NodeHeader::header_size, txl.encrypted_mapping, nullptr);
 #endif
-    auto size = NodeHeader::get_byte_size_from_header(addr);
+    // if we know the translation is inside the slab area, we don't need to check
+    // for anything beyond the header, and we don't need to check if decryption is needed
+    auto size = known_in_slab ? 8 : NodeHeader::get_byte_size_from_header(addr);
     bool crosses_mapping = offset + size > (1 << section_shift);
     // Move the limit on use of the existing primary mapping.
     // Take into account that another thread may attempt to change / have changed it concurrently,

src/realm/alloc.hpp

@@ -113,6 +113,9 @@ class Allocator {
     /// Calls do_translate().
     char* translate(ref_type ref) const noexcept;
 
+    /// Simpler version if we know the ref points inside the slab area
+    char* translate_in_slab(ref_type ref) const noexcept;
+
     /// Returns true if, and only if the object at the specified 'ref'
     /// is in the immutable part of the memory managed by this
     /// allocator. The method by which some objects become part of the
@@ -249,8 +252,8 @@ class Allocator {
     /// then entirely the responsibility of the caller that the memory
     /// is not modified by way of the returned memory pointer.
     virtual char* do_translate(ref_type ref) const noexcept = 0;
-    char* translate_critical(RefTranslation*, ref_type ref) const noexcept;
-    char* translate_less_critical(RefTranslation*, ref_type ref) const noexcept;
+    char* translate_critical(RefTranslation*, ref_type ref, bool known_in_slab = false) const noexcept;
+    char* translate_less_critical(RefTranslation*, ref_type ref, bool known_in_slab = false) const noexcept;
     virtual void get_or_add_xover_mapping(RefTranslation&, size_t, size_t, size_t) = 0;
     Allocator() noexcept;
     size_t get_section_index(size_t pos) const noexcept;
@@ -556,7 +559,8 @@ inline Allocator::Allocator() noexcept
 }
 
 // performance critical part of the translation process. Less critical code is in translate_less_critical.
-inline char* Allocator::translate_critical(RefTranslation* ref_translation_ptr, ref_type ref) const noexcept
+inline char* Allocator::translate_critical(RefTranslation* ref_translation_ptr, ref_type ref,
+                                           bool known_in_slab) const noexcept
 {
     size_t idx = get_section_index(ref);
     RefTranslation& txl = ref_translation_ptr[idx];
@@ -574,7 +578,7 @@ inline char* Allocator::translate_critical(RefTranslation* ref_translation_ptr,
         }
         else {
             // the lowest possible xover offset may grow concurrently, but that will be handled inside the call
-            return translate_less_critical(ref_translation_ptr, ref);
+            return translate_less_critical(ref_translation_ptr, ref, known_in_slab);
         }
     }
     realm::util::terminate("Invalid ref translation entry", __FILE__, __LINE__, txl.cookie, 0x1234567890, ref, idx);
@@ -592,6 +596,17 @@ inline char* Allocator::translate(ref_type ref) const noexcept
     }
 }
 
+inline char* Allocator::translate_in_slab(ref_type ref) const noexcept
+{
+    auto ref_translation_ptr = m_ref_translation_ptr.load(std::memory_order_acquire);
+    if (REALM_LIKELY(ref_translation_ptr)) {
+        return translate_critical(ref_translation_ptr, ref, true);
+    }
+    else {
+        return do_translate(ref);
+    }
+}
+
 
 } // namespace realm
 

src/realm/alloc_slab.cpp

@@ -235,7 +235,7 @@ MemRef SlabAlloc::do_alloc(size_t size)
 #endif
 
     char* addr = reinterpret_cast<char*>(entry);
-    REALM_ASSERT_EX(addr == translate(ref), addr, ref, get_file_path_for_assertions());
+    REALM_ASSERT_EX(addr == translate_in_slab(ref), addr, ref, get_file_path_for_assertions());
 
 #if REALM_ENABLE_ALLOC_SET_ZERO
     std::fill(addr, addr + size, 0);
@@ -300,6 +300,7 @@ SlabAlloc::FreeBlock* SlabAlloc::pop_freelist_entry(FreeList list)
 
 void SlabAlloc::FreeBlock::unlink()
 {
+    REALM_ASSERT_DEBUG(next != nullptr && prev != nullptr);
     auto _next = next;
     auto _prev = prev;
     _next->prev = prev;

src/realm/array.cpp

@@ -989,42 +989,18 @@ MemRef Array::clone(MemRef mem, Allocator& alloc, Allocator& target_alloc)
 MemRef Array::create(Type type, bool context_flag, WidthType width_type, size_t size, int_fast64_t value,
                      Allocator& alloc)
 {
-    REALM_ASSERT_7(value, ==, 0, ||, width_type, ==, wtype_Bits);
-    REALM_ASSERT_7(size, ==, 0, ||, width_type, !=, wtype_Ignore);
-
-    bool is_inner_bptree_node = false, has_refs = false;
-    switch (type) {
-        case type_Normal:
-            break;
-        case type_InnerBptreeNode:
-            is_inner_bptree_node = true;
-            has_refs = true;
-            break;
-        case type_HasRefs:
-            has_refs = true;
-            break;
-    }
-
+    REALM_ASSERT_DEBUG(value == 0 || width_type == wtype_Bits);
+    REALM_ASSERT_DEBUG(size == 0 || width_type != wtype_Ignore);
     int width = 0;
-    size_t byte_size_0 = header_size;
-    if (value != 0) {
-        width = int(bit_width(value));
-        byte_size_0 = calc_aligned_byte_size(size, width); // Throws
-    }
-    // Adding zero to Array::initial_capacity to avoid taking the
-    // address of that member
-    size_t byte_size = std::max(byte_size_0, initial_capacity + 0);
-    MemRef mem = alloc.alloc(byte_size); // Throws
-    char* header = mem.get_addr();
-
-    init_header(header, is_inner_bptree_node, has_refs, context_flag, width_type, width, size, byte_size);
-
+    if (value != 0)
+        width = static_cast<int>(bit_width(value));
+    auto mem = Node::create_node(size, alloc, context_flag, type, width_type, width);
     if (value != 0) {
+        const auto header = mem.get_addr();
         char* data = get_data_from_header(header);
         size_t begin = 0, end = size;
         REALM_TEMPEX(fill_direct, width, (data, begin, end, value));
     }
-
     return mem;
 }
 

src/realm/array.hpp

@@ -485,7 +485,7 @@ class Array : public Node, public ArrayParent {
     /// It is an error to specify a non-zero value unless the width
     /// type is wtype_Bits. It is also an error to specify a non-zero
     /// size if the width type is wtype_Ignore.
-    static MemRef create(Type, bool context_flag, WidthType, size_t size, int_fast64_t value, Allocator&);
+    static MemRef create(Type, bool, WidthType, size_t, int_fast64_t, Allocator&);
 
     // Overriding method in ArrayParent
     void update_child_ref(size_t, ref_type) override;
@@ -938,8 +938,7 @@ inline void Array::adjust(size_t begin, size_t end, int_fast64_t diff)
 inline size_t Array::get_byte_size() const noexcept
 {
     const char* header = get_header_from_data(m_data);
-    WidthType wtype = Node::get_wtype_from_header(header);
-    size_t num_bytes = NodeHeader::calc_byte_size(wtype, m_size, m_width);
+    size_t num_bytes = NodeHeader::get_byte_size_from_header(header);
 
     REALM_ASSERT_7(m_alloc.is_read_only(m_ref), ==, true, ||, num_bytes, <=, get_capacity_from_header(header));
 

src/realm/array_basic_tpl.hpp

@@ -40,16 +40,12 @@ inline MemRef BasicArray<T>::create_array(size_t init_size, Allocator& allocator
     // Adding zero to Array::initial_capacity to avoid taking the
     // address of that member
     size_t byte_size = std::max(byte_size_0, Node::initial_capacity + 0); // Throws
-
-    MemRef mem = allocator.alloc(byte_size); // Throws
-
-    bool is_inner_bptree_node = false;
-    bool has_refs = false;
-    bool context_flag = false;
-    int width = sizeof(T);
-    init_header(mem.get_addr(), is_inner_bptree_node, has_refs, context_flag, wtype_Multiply, width, init_size,
-                byte_size);
-
+    MemRef mem = allocator.alloc(byte_size);                              // Throws
+    uint8_t flags = 0;
+    const int width = sizeof(T) * 8; // element width is in bits now
+    const auto header = mem.get_addr();
+    init_header(header, Encoding::WTypMult, flags, width, init_size);
+    set_capacity_in_header(byte_size, header);
     return mem;
 }
 

src/realm/node.cpp

@@ -32,11 +32,24 @@ MemRef Node::create_node(size_t size, Allocator& alloc, bool context_flag, Type
     size_t byte_size = std::max(byte_size_0, size_t(initial_capacity));
 
     MemRef mem = alloc.alloc(byte_size); // Throws
-    char* header = mem.get_addr();
-
-    init_header(header, type == type_InnerBptreeNode, type != type_Normal, context_flag, width_type, width, size,
-                byte_size);
-
+    const auto header = mem.get_addr();
+    REALM_ASSERT_DEBUG(width_type != WidthType::wtype_Extend);
+    Encoding encoding{static_cast<int>(width_type)};
+
+    uint8_t flags = 0;
+    if (type == type_InnerBptreeNode)
+        flags |= static_cast<uint8_t>(Flags::InnerBPTree) | static_cast<uint8_t>(Flags::HasRefs);
+    if (type != type_Normal)
+        flags |= static_cast<uint8_t>(Flags::HasRefs);
+    if (context_flag)
+        flags |= static_cast<uint8_t>(Flags::Context);
+    // width must be passed to init_header in bits, but for wtype_Multiply and wtype_Ignore
+    // it is provided by the caller of this function in bytes, so convert to bits
+    if (width_type != wtype_Bits)
+        width = width * 8;
+
+    init_header(header, encoding, flags, width, size);
+    set_capacity_in_header(byte_size, header);
     return mem;
 }
 
@@ -69,17 +82,20 @@ size_t Node::calc_item_count(size_t bytes, size_t width) const noexcept
 void Node::alloc(size_t init_size, size_t new_width)
 {
     REALM_ASSERT(is_attached());
-
+    char* header = get_header_from_data(m_data);
+    REALM_ASSERT(!wtype_is_extended(header));
     size_t needed_bytes = calc_byte_len(init_size, new_width);
     // this method is not public and callers must (and currently do) ensure that
     // needed_bytes are never larger than max_array_payload.
     REALM_ASSERT_RELEASE(init_size <= max_array_size);
 
-    if (is_read_only())
+    if (is_read_only()) {
         do_copy_on_write(needed_bytes);
+        // header has changed after copy on write if the array was compressed
+        header = get_header_from_data(m_data);
+    }
 
     REALM_ASSERT(!m_alloc.is_read_only(m_ref));
-    char* header = get_header_from_data(m_data);
     size_t orig_capacity_bytes = get_capacity_from_header(header);
     size_t orig_width = get_width_from_header(header);
 
@@ -114,18 +130,28 @@ void Node::alloc(size_t init_size, size_t new_width)
         // this array instance in a corrupt state
         update_parent(); // Throws
     }
-
-    // Update header
+    // update width (important when we convert from normal uncompressed array into compressed format)
     if (new_width != orig_width) {
         set_width_in_header(int(new_width), header);
     }
     set_size_in_header(init_size, header);
     m_size = init_size;
 }
 
+void Node::destroy() noexcept
+{
+    if (!is_attached())
+        return;
+    char* header = get_header_from_data(m_data);
+    m_alloc.free_(m_ref, header);
+    m_data = nullptr;
+}
+
 void Node::do_copy_on_write(size_t minimum_size)
 {
     const char* header = get_header_from_data(m_data);
+    // only type A arrays should be allowed during copy on write
+    REALM_ASSERT(!wtype_is_extended(header));
 
     // Calculate size in bytes
     size_t array_size = calc_byte_size(get_wtype_from_header(header), m_size, get_width_from_header(header));
@@ -140,7 +166,6 @@ void Node::do_copy_on_write(size_t minimum_size)
     const char* old_end = header + array_size;
     char* new_begin = mref.get_addr();
     realm::safe_copy_n(old_begin, old_end - old_begin, new_begin);
-
     ref_type old_ref = m_ref;
 
     // Update internal data
@@ -150,7 +175,6 @@ void Node::do_copy_on_write(size_t minimum_size)
     // Update capacity in header. Uses m_data to find header, so
     // m_data must be initialized correctly first.
     set_capacity_in_header(new_size, new_begin);
-
     update_parent();
 
 #if REALM_ENABLE_MEMDEBUG

src/realm/node.hpp

@@ -114,7 +114,7 @@ class Node : public NodeHeader {
     {
     }
 
-    virtual ~Node() {}
+    virtual ~Node() = default;
 
     /**************************** Initializers *******************************/
 
@@ -123,10 +123,10 @@ class Node : public NodeHeader {
     char* init_from_mem(MemRef mem) noexcept
     {
         char* header = mem.get_addr();
+        REALM_ASSERT_DEBUG(!wtype_is_extended(header));
         m_ref = mem.get_ref();
         m_data = get_data_from_header(header);
         m_size = get_size_from_header(header);
-
         return header;
     }
 
@@ -212,14 +212,7 @@ class Node : public NodeHeader {
     /// children of that array. See non-static destroy_deep() for an
     /// alternative. If this accessor is already in the detached state, this
     /// function has no effect (idempotency).
-    void destroy() noexcept
-    {
-        if (!is_attached())
-            return;
-        char* header = get_header_from_data(m_data);
-        m_alloc.free_(m_ref, header);
-        m_data = nullptr;
-    }
+    void destroy() noexcept;
 
     /// Shorthand for `destroy(MemRef(ref, alloc), alloc)`.
     static void destroy(ref_type ref, Allocator& alloc) noexcept
@@ -234,7 +227,6 @@ class Node : public NodeHeader {
         alloc.free_(mem);
     }
 
-
     /// Setting a new parent affects ownership of the attached array node, if
     /// any. If a non-null parent is specified, and there was no parent
     /// originally, then the caller passes ownership to the parent, and vice
@@ -245,6 +237,7 @@ class Node : public NodeHeader {
         m_parent = parent;
         m_ndx_in_parent = ndx_in_parent;
     }
+
     void set_ndx_in_parent(size_t ndx) noexcept
     {
         m_ndx_in_parent = ndx;
@@ -333,8 +326,6 @@ class Node : public NodeHeader {
     // Includes array header. Not necessarily 8-byte aligned.
     virtual size_t calc_byte_len(size_t num_items, size_t width) const;
     virtual size_t calc_item_count(size_t bytes, size_t width) const noexcept;
-    static void init_header(char* header, bool is_inner_bptree_node, bool has_refs, bool context_flag,
-                            WidthType width_type, int width, size_t size, size_t capacity) noexcept;
 
 private:
     friend class NodeTree;
@@ -362,22 +353,6 @@ class ArrayPayload {
     virtual void set_spec(Spec*, size_t) const {}
 };
 
-
-inline void Node::init_header(char* header, bool is_inner_bptree_node, bool has_refs, bool context_flag,
-                              WidthType width_type, int width, size_t size, size_t capacity) noexcept
-{
-    // Note: Since the header layout contains unallocated bit and/or
-    // bytes, it is important that we put the entire header into a
-    // well defined state initially.
-    std::fill(header, header + header_size, 0);
-    set_is_inner_bptree_node_in_header(is_inner_bptree_node, header);
-    set_hasrefs_in_header(has_refs, header);
-    set_context_flag_in_header(context_flag, header);
-    set_wtype_in_header(width_type, header);
-    set_width_in_header(width, header);
-    set_size_in_header(size, header);
-    set_capacity_in_header(capacity, header);
-}
 } // namespace realm
 
 #endif /* REALM_NODE_HPP */