Log::Metadata should put parent entries first on extend (like Hash#merge) (#16098)

`Log::Metadata` wrote its entries in the beginning of the allocated memory for all entries. When extending, parent entries were prepended to the memory.

With this change, entries are written to the end of allocated memory to make it possible to write parent entries first when extending the `Metadata` instance. `#defrag` moves the local entries to the end of the parent entries or overwrites any existing parent entry for the current key.

Author: spuun

spec/std/log/metadata_spec.cr

@@ -63,7 +63,7 @@ describe Log::Metadata do
 
   it "json" do
     m({a: 1}).to_json.should eq(%({"a":1}))
-    m({a: 1, b: 1}).extend({b: 2}).to_json.should eq(%({"b":2,"a":1}))
+    m({a: 1, b: 1}).extend({b: 2}).to_json.should eq(%({"a":1,"b":2}))
   end
 
   it "defrags" do
@@ -72,17 +72,37 @@ describe Log::Metadata do
 
     md.@size.should eq(1)
     md.@max_total_size.should eq(4)
-    md.@overridden_size.should eq(1)
     md.@parent.should be(parent)
 
     md.should eq(m({a: 3, b: 2}))
 
     md.@size.should eq(2)
     md.@max_total_size.should eq(2)
-    md.@overridden_size.should eq(1)
     md.@parent.should be_nil
   end
 
+  it "defrags deep" do
+    grandparent = m({x: 100, y: 200, z: 300})
+    parent = grandparent.extend({x: 101, a: 1, b: 2})
+    md = parent.extend({x: 102, b: 22, c: 3})
+
+    md.should eq(m({x: 102, y: 200, z: 300, a: 1, b: 22, c: 3}))
+  end
+
+  it "[] find parent entry after defrag" do
+    grandparent = m({x: 100, y: 200, z: 300})
+    parent = grandparent.extend({x: 101, a: 1, b: 2})
+    md = parent.extend({x: 102, b: 22, c: 3})
+
+    _ = md.to_a
+
+    md[:y].should eq(200)
+    md[:z].should eq(300)
+    md[:x].should eq(102)
+    md[:a].should eq(1)
+    md[:b].should eq(22)
+  end
+
   it "[]" do
     md = m({a: 1, b: 2}).extend({a: 3})
 

src/log/metadata.cr

@@ -20,13 +20,14 @@ class Log::Metadata
   # When the metadata is defragmented max_total_size will be updated with size
   protected getter max_total_size : Int32
   @max_total_size = uninitialized Int32
-  # How many entries are potentially overridden from parent (ie: initial entries.size)
-  @overridden_size = uninitialized Int32
   # How many entries are stored from @first.
-  # Initially are @overridden_size, the one explicitly overridden in entries argument.
   # When the metadata is defragmented @size will be increased up to
   # the actual number of entries resulting from merging the parent
   @size = uninitialized Int32
+  # Number of parent elements we've copied on defrag. Used to iterate parent
+  # entries first in #each.
+  @parent_size = uninitialized Int32
+
   # @first needs to be the last ivar of Metadata. The entries are allocated together with self
   @first = uninitialized Entry
 
@@ -42,7 +43,8 @@ class Log::Metadata
   end
 
   protected def setup(@parent : Metadata?, entries : NamedTuple | Hash)
-    @size = @overridden_size = entries.size
+    @size = entries.size
+    @parent_size = 0
     @max_total_size = @size + (@parent.try(&.max_total_size) || 0)
     ptr_entries = pointerof(@first)
 
@@ -91,43 +93,44 @@ class Log::Metadata
   # will be recomputed, but the result should be the same.
   #
   # * @parent.nil? signals if the defrag is needed/done
-  # * The values of @overridden_size, pointerof(@first) are never changed
+  # * The value of pointerof(@first) never changes
   # * @parent is set at the very end of the method
   protected def defrag
     parent = @parent
     return if parent.nil?
 
-    total_size = @overridden_size
     ptr_entries = pointerof(@first)
-    next_free_entry = ptr_entries + @overridden_size
+    next_free_entry = ptr_entries + @size
+    total_size = @size
 
+    # Copy parent entries that ain't overwritten
+    parent_size = 0
     parent.each do |(key, value)|
-      overridden = false
-      @overridden_size.times do |i|
-        if ptr_entries[i][:key] == key
-          overridden = true
-          break
-        end
-      end
-
-      unless overridden
-        next_free_entry.value = {key: key, value: value}
-        next_free_entry += 1
-        total_size += 1
-      end
+      overwritten = Slice.new(ptr_entries, @size).any? { |entry| entry[:key] == key }
+      next if overwritten
+      next_free_entry.value = {key: key, value: value}
+      parent_size += 1
+      next_free_entry += 1
+      total_size += 1
     end
 
     @size = total_size
     @max_total_size = total_size
+    @parent_size = parent_size
     @parent = nil
   end
 
   def each(& : {Symbol, Value} ->)
     defrag
     ptr_entries = pointerof(@first)
+    parent_size = @parent_size
+    local_size = @size - parent_size
 
-    @size.times do |i|
-      entry = ptr_entries[i]
+    Slice.new(ptr_entries + local_size, parent_size).each do |entry|
+      yield({entry[:key], entry[:value]})
+    end
+
+    Slice.new(ptr_entries, local_size).each do |entry|
       yield({entry[:key], entry[:value]})
     end
   end