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

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
 
     [email protected] eq(1)
     md.@max_total_size.should eq(4)
-    md.@overridden_size.should eq(1)
     [email protected] be(parent)
 
     md.should eq(m({a: 3, b: 2}))
 
     [email protected] eq(2)
     md.@max_total_size.should eq(2)
-    md.@overridden_size.should eq(1)
     [email protected] 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