summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJohn Keiser <john@johnkeiser.com>2015-06-29 16:20:15 -0600
committerJohn Keiser <john@johnkeiser.com>2015-06-30 11:18:41 -0600
commit7f2efa0bdabee2745b9b61725d88944bedd44052 (patch)
tree95dc93c7059d18ac90b173fca227dd284f2131cc
parent2bf7afdba5496f5f3e7ba065650c6db5522fe8c1 (diff)
downloadchef-7f2efa0bdabee2745b9b61725d88944bedd44052.tar.gz
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.
-rw-r--r--lib/chef/node_map.rb31
-rw-r--r--lib/chef/platform/priority_map.rb8
-rw-r--r--lib/chef/provider_resolver.rb53
-rw-r--r--lib/chef/resource.rb2
-rw-r--r--lib/chef/resource_resolver.rb41
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