Optimize logic on class initialize so it doesn't fall into the

"look at all classes to see if they provide the DSL" else clause
when you're just setting resource_name (which happens on every
class). Perf fix for tests.

5 files changed, 80 insertions, 55 deletions

diff --git a/lib/chef/node_map.rb b/lib/chef/node_map.rb
index edf0fd689a..1ce52818b2 100644
--- a/lib/chef/node_map.rb
+++ b/lib/chef/node_map.rb
@@ -20,13 +20,6 @@ class Chef
   class NodeMap
 
     #
-    # Create a new NodeMap
-    #
-    def initialize
-      @map = {}
-    end
-
-    #
     # Set a key/value pair on the map with a filter.  The filter must be true
     # when applied to the node in order to retrieve the value.
     #
@@ -55,18 +48,18 @@ class Chef
       # The map is sorted in order of preference already; we just need to find
       # our place in it (just before the first value with the same preference level).
       insert_at = nil
-      @map[key] ||= []
-      @map[key].each_with_index do |matcher,index|
+      map[key] ||= []
+      map[key].each_with_index do |matcher,index|
         cmp = compare_matchers(key, new_matcher, matcher)
         insert_at ||= index if cmp && cmp <= 0
       end
       if insert_at
-        @map[key].insert(insert_at, new_matcher)
+        map[key].insert(insert_at, new_matcher)
       else
-        @map[key] << new_matcher
+        map[key] << new_matcher
       end
-      insert_at ||= @map[key].size - 1
-      @map
+      insert_at ||= map[key].size - 1
+      map
     end
 
     #
@@ -100,8 +93,8 @@ class Chef
     #
     def list(node, key, canonical: nil)
       raise ArgumentError, "first argument must be a Chef::Node" unless node.is_a?(Chef::Node) || node.nil?
-      return [] unless @map.has_key?(key)
-      @map[key].select do |matcher|
+      return [] unless map.has_key?(key)
+      map[key].select do |matcher|
         node_matches?(node, matcher) && canonical_matches?(canonical, matcher)
       end.map { |matcher| matcher[:value] }
     end
@@ -110,11 +103,11 @@ class Chef
     # @return remaining
     # @api private
     def delete_canonical(key, value)
-      remaining = @map[key]
+      remaining = map[key]
       if remaining
         remaining.delete_if { |matcher| matcher[:canonical] && Array(matcher[:value]) == Array(value) }
         if remaining.empty?
-          @map.delete(key)
+          map.delete(key)
           remaining = nil
         end
       end
@@ -222,5 +215,9 @@ class Chef
       end
       cmp
     end
+
+    def map
+      @map ||= {}
+    end
   end
 end
diff --git a/lib/chef/platform/priority_map.rb b/lib/chef/platform/priority_map.rb
index d559eece78..73554eafe1 100644
--- a/lib/chef/platform/priority_map.rb
+++ b/lib/chef/platform/priority_map.rb
@@ -6,7 +6,7 @@ class Chef
       def priority(resource_name, priority_array, *filter)
         set_priority_array(resource_name.to_sym, priority_array, *filter)
       end
-      
+
       # @api private
       def get_priority_array(node, key)
         get(node, key)
@@ -24,6 +24,12 @@ class Chef
         list(node, key, **filters).flatten(1).uniq
       end
 
+      # @api private
+      def includes_handler?(key, handler)
+        return false if !map.has_key?(key)
+        map[key].any? { |m| h = m[:value]; h.is_a?(Array) ? h.include?(handler) : h == handler }
+      end
+
       #
       # Priority maps have one extra precedence: priority arrays override "provides,"
       # and "provides" lines with identical filters sort by class name (ascending).
diff --git a/lib/chef/provider_resolver.rb b/lib/chef/provider_resolver.rb
index f8e161f721..6c49a39ba4 100644
--- a/lib/chef/provider_resolver.rb
+++ b/lib/chef/provider_resolver.rb
@@ -62,12 +62,19 @@ class Chef
         maybe_chef_platform_lookup(resource)
     end
 
+    # Does NOT call provides? on the resource (it is assumed this is being
+    # called *from* provides?).
     def provided_by?(provider_class)
       potential_handlers.include?(provider_class)
     end
 
+    def self.includes_handler?(resource_name, provider_class)
+      priority_map.includes_handler?(resource_name, provider_class)
+    end
+
     def enabled_handlers
-      @enabled_handlers ||= potential_handlers.select { |handler| handler.method(:provides?).owner == Chef::Provider || handler.provides?(node, resource) }
+      @enabled_handlers ||=
+        potential_handlers.select { |handler| !overrode_provides?(handler) || handler.provides?(node, resource) }
     end
 
     # TODO deprecate this and allow actions to be passed as a filter to
@@ -94,15 +101,16 @@ class Chef
     def maybe_dynamic_provider_resolution(resource, action)
       Chef::Log.debug "Providers for generic #{resource.resource_name} resource enabled on node include: #{enabled_handlers}"
 
-      if supported_handlers.empty?
+      handlers = supported_handlers
+      if handlers.empty?
         # if none of the providers specifically support the resource, we still need to pick one of the providers that are
         # enabled on the node to handle the why-run use case. FIXME we should only do this in why-run mode then.
         Chef::Log.debug "No providers responded true to `supports?` for action #{action} on resource #{resource}, falling back to enabled handlers so we can return something anyway."
-        handler = enabled_handlers.first
-      else
-        handler = supported_handlers.first
+        handlers = enabled_handlers
       end
 
+      handler = handlers.first
+
       if handler
         Chef::Log.debug "Provider for action #{action} on resource #{resource} is #{handler}"
       else
@@ -117,10 +125,18 @@ class Chef
       Chef::Platform.find_provider_for_node(node, resource)
     end
 
+    def self.priority_map
+      Chef::Platform::ProviderPriorityMap.instance
+    end
+
     def priority_map
       Chef::Platform::ProviderPriorityMap.instance
     end
 
+    def overrode_provides?(handler)
+      handler.method(:provides?).owner != Chef::Provider.method(:provides?).owner
+    end
+
     module Deprecated
       # return a deterministically sorted list of Chef::Provider subclasses
       def providers
@@ -129,26 +145,19 @@ class Chef
 
       def enabled_handlers
         @enabled_handlers ||= begin
-          handlers = potential_handlers
-          # If there are no potential handlers for this node (nobody called provides)
-          # then we search through all classes and call provides in case someone
-          # defined a provides? method that returned true in spite of provides
-          # not being called
+          handlers = super
           if handlers.empty?
-            warn = true
-            handlers = providers
-          end
-          handlers.select do |handler|
-            if handler.method(:provides?).owner == Chef::Provider
-              true
-            elsif handler.provides?(node, resource)
-              if warn
-                Chef::Log.deprecation("#{handler}.provides? returned true when asked if it provides DSL #{resource.resource_name}, but provides #{resource.resource_name.to_sym.inspect} was never called!")
-                Chef::Log.deprecation("In Chef 13, this will break: you must call provides to mark the names you provide, even if you also override provides? yourself.")
-              end
-              true
+            # Look through all providers, and find ones that return true to provides.
+            # Don't bother with ones that don't override provides?, since they
+            # would have been in enabled_handlers already if that were so. (It's a
+            # perf concern otherwise.)
+            handlers = providers.select { |handler| overrode_provides?(handler) && handler.provides?(node, resource) }
+            handlers.each do |handler|
+              Chef::Log.deprecation("#{handler}.provides? returned true when asked if it provides DSL #{resource.resource_name}, but provides #{resource.resource_name.inspect} was never called!")
+              Chef::Log.deprecation("In Chef 13, this will break: you must call provides to mark the names you provide, even if you also override provides? yourself.")
             end
           end
+          handlers
         end
       end
     end
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 9510351d16..7965068037 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -1017,7 +1017,7 @@ class Chef
         if name
           name = name.to_sym
           # If our class is not already providing this name, provide it.
-          if !Chef::ResourceResolver.list(name).include?(self)
+          if !Chef::ResourceResolver.includes_handler?(name, self)
             provides name, canonical: true
           end
           @resource_name = name
diff --git a/lib/chef/resource_resolver.rb b/lib/chef/resource_resolver.rb
index 11121df04a..e138ad0d51 100644
--- a/lib/chef/resource_resolver.rb
+++ b/lib/chef/resource_resolver.rb
@@ -56,6 +56,7 @@ class Chef
     attr_reader :resource_name
     # @api private
     def resource
+      raise 'omg'
       Chef::Log.deprecation("Chef::ResourceResolver.resource deprecated.  Use resource_name instead.")
       resource_name
     end
@@ -105,13 +106,28 @@ class Chef
     #
     # Whether this DSL is provided by the given resource_class.
     #
+    # Does NOT call provides? on the resource (it is assumed this is being
+    # called *from* provides?).
+    #
     # @api private
     def provided_by?(resource_class)
       potential_handlers.include?(resource_class)
     end
 
+    #
+    # Whether the given handler attempts to provide the resource class at all.
+    #
+    # @api private
+    def self.includes_handler?(resource_name, resource_class)
+      priority_map.includes_handler?(resource_name, resource_class)
+    end
+
     protected
 
+    def self.priority_map
+      Chef::Platform::ResourcePriorityMap.instance
+    end
+
     def priority_map
       Chef::Platform::ResourcePriorityMap.instance
     end
@@ -122,7 +138,11 @@ class Chef
     end
 
     def enabled_handlers
-      potential_handlers.select { |handler| handler.method(:provides?).owner == Chef::Resource || handler.provides?(node, resource_name) }
+      potential_handlers.select { |handler| !overrode_provides?(handler) || handler.provides?(node, resource_name) }
+    end
+
+    def overrode_provides?(handler)
+      handler.method(:provides?).owner != Chef::Resource.method(:provides?).owner
     end
 
     module Deprecated
@@ -133,22 +153,15 @@ class Chef
 
       def enabled_handlers
         @enabled_handlers ||= begin
-          handlers = potential_handlers
+          handlers = super
           if handlers.empty?
-            warn = true
-            handlers = resources
-          end
-          handlers.select do |handler|
-            if handler.method(:provides?).owner == Chef::Resource
-              true
-            elsif handler.provides?(node, resource_name)
-              if warn
-                Chef::Log.deprecation("#{handler}.provides? returned true when asked if it provides DSL #{resource_name}, but provides #{resource_name.inspect} was never called!")
-                Chef::Log.deprecation("In Chef 13, this will break: you must call provides to mark the names you provide, even if you also override provides? yourself.")
-              end
-              true
+            handlers = resources.select { |handler| overrode_provides?(handler) && handler.provides?(node, resource_name) }
+            handlers.each do |handler|
+              Chef::Log.deprecation("#{handler}.provides? returned true when asked if it provides DSL #{resource_name}, but provides #{resource_name.inspect} was never called!")
+              Chef::Log.deprecation("In Chef 13, this will break: you must call provides to mark the names you provide, even if you also override provides? yourself.")
             end
           end
+          handlers
         end
       end
     end