Merge pull request #12744 from chef/lcg/fix-attribute-performance-issues3

3 files changed, 32 insertions, 7 deletions

diff --git a/chef-utils/lib/chef-utils/mash.rb b/chef-utils/lib/chef-utils/mash.rb
index 484e172b33..08f4e62b59 100644
--- a/chef-utils/lib/chef-utils/mash.rb
+++ b/chef-utils/lib/chef-utils/mash.rb
@@ -102,6 +102,14 @@ module ChefUtils
       alias_method :regular_update, :update
     end
 
+    unless method_defined?(:regular_clear)
+      alias_method :regular_clear, :clear
+    end
+
+    unless method_defined?(:regular_delete)
+      alias_method :regular_delete, :delete
+    end
+
     # @param key<Object> The key to set.
     # @param value<Object>
     #   The value to set the key to.
diff --git a/lib/chef/node/attribute.rb b/lib/chef/node/attribute.rb
index 29b60a98d5..277f1e0727 100644
--- a/lib/chef/node/attribute.rb
+++ b/lib/chef/node/attribute.rb
@@ -450,17 +450,34 @@ class Chef
       # method-style access to attributes (has to come after the prepended ImmutablizeHash)
 
       def read(*path)
-        merged_attributes.read(*path)
+        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
+          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)
-        merged_attributes.read!(*path)
+        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
+          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)
-        merged_attributes.exist?(*path)
+        if path[0].nil?
+          true
+        else
+          self[path[0]] unless path[0].nil? # force deep_merge_cache key construction if necessary
+          deep_merge_cache.exist?(*path)
+        end
       end
 
       def write(level, *args, &block)
diff --git a/lib/chef/node/mixin/deep_merge_cache.rb b/lib/chef/node/mixin/deep_merge_cache.rb
index e88e079895..10d14ed84b 100644
--- a/lib/chef/node/mixin/deep_merge_cache.rb
+++ b/lib/chef/node/mixin/deep_merge_cache.rb
@@ -28,7 +28,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
@@ -37,9 +37,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
 
@@ -51,7 +51,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
         end