Convert the deep merge cache to an ImmutableMash

This makes it so that callers can call the deep merge cache
more or less directly so that we're not hanging onto a weird
Hash-of-ImmutableMashes structure.

Care still needs to be taken to consider the fact that:

- the deep merge cache may not be merged for a given keys since
  it is lazily created, so we need to hit the top level accessor
  once to make sure the key is merged before we access it.

- the top-level "give me it all" case where the path is totally
  empty is still handled by doing a manual deep merge of everything.

Once that is done we can call APIs like read/read!/exist? directly
on the ImmutableMash and have fewer edge cases to deal with.

This has the same perf speedup that the prior approach does and
keeps attribute mangling down to 1% of my runtime on my own cookbooks

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

2 files changed, 19 insertions, 45 deletions

diff --git a/lib/chef/node/attribute.rb b/lib/chef/node/attribute.rb
index 9be5293815..d235ce4faa 100644
--- a/lib/chef/node/attribute.rb
+++ b/lib/chef/node/attribute.rb
@@ -38,7 +38,7 @@ class Chef
 
       include Immutablize
       # FIXME:  what is include Enumerable doing up here, when down below we delegate
-      # most of the Enumerable/Hash things to the underlying merged ImmutableMash.  That
+      # most of the Enumerable/Hash things to the underlying merged ImmutableHash.  That
       # is, in fact, the correct, thing to do, while including Enumerable to try to create
       # a hash-like API gets lots of things wrong because of the difference between the
       # Hash `each do |key, value|` vs the Array-like `each do |value|` API that Enumerable
@@ -452,59 +452,33 @@ class Chef
       # method-style access to attributes (has to come after the prepended ImmutablizeHash)
 
       def read(*path)
-        rest = path.dup
-        first = rest.shift
-
-        # this will have terrible performance, but it is the only way to get at the entire
-        # merged ImmutableMash top level object.
-        return merged_attributes if first.nil?
-
-        obj = self[first]
-        if obj.is_a?(ImmutableMash) || obj.is_a?(ImmutableArray)
-          obj.read(*rest)
+        if path[0].nil?
+          Chef::Log.warn "Calling node.read() without any path argument is very slow, probably a bug, and should be avoided"
+          merged_attributes.read(*path) # re-merges everything, slow edge case
         else
-          obj
+          self[path[0]] unless path[0].nil? # force deep_merge_cache key construction if necessary
+          deep_merge_cache.read(*path)
         end
       end
 
       alias :dig :read
 
       def read!(*path)
-        rest = path.dup
-        first = rest.shift
-
-        # this will have terrible performance, but it is the only way to get at the entire
-        # merged ImmutableMash top level object.
-        return merged_attributes if first.nil?
-
-        raise Chef::Exceptions::NoSuchAttribute.new(path.join ".") unless key?(first)
-
-        obj = self[first]
-
-        return obj if rest.empty?
-
-        if obj.is_a?(ImmutableMash) || obj.is_a?(ImmutableArray)
-          obj.read!(*rest)
+        if path[0].nil?
+          Chef::Log.warn "Calling node.read!() without any path argument is very slow, probably a bug, and should be avoided"
+          merged_attributes.read!(*path) # re-merges everything, slow edge case
         else
-          raise Chef::Exceptions::NoSuchAttribute.new(path.join ".")
+          self[path[0]] unless path[0].nil? # force deep_merge_cache key construction if necessary
+          deep_merge_cache.read!(*path)
         end
       end
 
       def exist?(*path)
-        rest = path.dup
-        first = rest.shift
-
-        return true if first.nil?
-
-        return false unless key?(first)
-
-        return true if rest.empty?
-
-        obj = self[first]
-        if obj.is_a?(ImmutableMash) || obj.is_a?(ImmutableArray)
-          obj.exist?(*rest)
+        if path[0].nil?
+          true
         else
-          false
+          self[path[0]] unless path[0].nil? # force deep_merge_cache key construction if necessary
+          deep_merge_cache.exist?(*path)
         end
       end
 
diff --git a/lib/chef/node/mixin/deep_merge_cache.rb b/lib/chef/node/mixin/deep_merge_cache.rb
index 8978d77ea0..be16197850 100644
--- a/lib/chef/node/mixin/deep_merge_cache.rb
+++ b/lib/chef/node/mixin/deep_merge_cache.rb
@@ -30,7 +30,7 @@ class Chef
           @merged_attributes = nil
           @combined_override = nil
           @combined_default = nil
-          @deep_merge_cache = {}
+          @deep_merge_cache = Chef::Node::ImmutableMash.new
         end
 
         # Invalidate a key in the deep_merge_cache.  If called with nil, or no arg, this will invalidate
@@ -39,9 +39,9 @@ class Chef
         # must invalidate the entire cache and re-deep-merge the entire node object.
         def reset_cache(path = nil)
           if path.nil?
-            deep_merge_cache.clear
+            deep_merge_cache.regular_clear
           else
-            deep_merge_cache.delete(path.to_s)
+            deep_merge_cache.regular_delete(path.to_s)
           end
         end
 
@@ -53,7 +53,7 @@ class Chef
                   deep_merge_cache[key.to_s]
                 else
                   # save all the work of computing node[key]
-                  deep_merge_cache[key.to_s] = merged_attributes(key)
+                  deep_merge_cache.internal_set(key.to_s, merged_attributes(key))
                 end
           ret = ret.call while ret.is_a?(::Chef::DelayedEvaluator)
           ret