Skip to content

Commit

Permalink
Fix #4461 - attempt to fix another performance issue
Browse files Browse the repository at this point in the history
During a profiling of a 2.6.1rc1 puppet master, I found that we
spend a lot of time and efforts in Puppet::Util::Autoload#module_directories.
Since this method is doing a bunch of filesystem access, this process is
slow.

In fact each time we were evaluating a resource or trying to find if a given
resource was a builtin type we ended up scanning the whole module directories
for the given environment.

This patch attempts to fix this performance issue by caching the module_directories
output for the either the time of the compilation or an agent configurer
run (since this stuff looks like to be shared for both compilation and
catalog evaluation).

With this patch, my compilation time for 2k resources went from 5.91s to
3.71s (second run each time to allievate parsing time)..

Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>

Edited to fix a typo [#4434]

Signed-off-by: Jesse Wolfe <jes5199@gmail.com>
  • Loading branch information
Brice Figureau authored and markus committed Aug 3, 2010
1 parent 2c21fae commit f54d843
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 5 deletions.
4 changes: 4 additions & 0 deletions lib/puppet/configurer.rb
Expand Up @@ -158,6 +158,10 @@ def run(options = {})
return
end
ensure
# Make sure we forget the retained module_directories of any autoload
# we might have used.
Thread.current[:env_module_directories] = nil

# Now close all of our existing http connections, since there's no
# reason to leave them lying open.
Puppet::Network::HttpPool.clear_http_instances
Expand Down
1 change: 1 addition & 0 deletions lib/puppet/parser/compiler.rb
Expand Up @@ -23,6 +23,7 @@ def self.compile(node)
# We get these from the environment and only cache them in a thread
# variable for the duration of the compilation.
Thread.current[:known_resource_types] = nil
Thread.current[:env_module_directories] = nil
end

attr_reader :node, :facts, :collections, :catalog, :node_scope, :resources, :relationships
Expand Down
22 changes: 17 additions & 5 deletions lib/puppet/util/autoload.rb
Expand Up @@ -131,11 +131,23 @@ def module_directories(env=nil)
# We have to require this late in the process because otherwise we might have
# load order issues.
require 'puppet/node/environment'
Puppet::Node::Environment.new(env).modulepath.collect do |dir|
Dir.entries(dir).reject { |f| f =~ /^\./ }.collect { |f| File.join(dir, f) }
end.flatten.collect { |d| [File.join(d, "plugins"), File.join(d, "lib")] }.flatten.find_all do |d|
FileTest.directory?(d)
end

real_env = Puppet::Node::Environment.new(env)

# We're using a per-thread cache of said module directories, so that
# we don't scan the filesystem each time we try to load something with
# this autoload instance. But since we don't want to cache for the eternity
# this env_module_directories gets reset after the compilation on the master.
# This is also reset after an agent ran.
# One of the side effect of this change is that this module directories list will be
# shared among all autoload that we have running at a time. But that won't be an issue
# as by definition those directories are shared by all autoload.
Thread.current[:env_module_directories] ||= {}
Thread.current[:env_module_directories][real_env] ||= real_env.modulepath.collect do |dir|
Dir.entries(dir).reject { |f| f =~ /^\./ }.collect { |f| File.join(dir, f) }
end.flatten.collect { |d| [File.join(d, "plugins"), File.join(d, "lib")] }.flatten.find_all do |d|
FileTest.directory?(d)
end
end

def search_directories(env=nil)
Expand Down

0 comments on commit f54d843

Please sign in to comment.