Fix #4461 - attempt to fix another performance issue

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>

3 files changed, 22 insertions, 5 deletions

diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb
index 327955b03..31d31c2d2 100644
--- a/lib/puppet/configurer.rb
+++ b/lib/puppet/configurer.rb
@@ -158,6 +158,10 @@ class Puppet::Configurer
       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
diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
index 61bb13cb6..98cad952c 100644
--- a/lib/puppet/parser/compiler.rb
+++ b/lib/puppet/parser/compiler.rb
@@ -23,6 +23,7 @@ class Puppet::Parser::Compiler
     # 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
diff --git a/lib/puppet/util/autoload.rb b/lib/puppet/util/autoload.rb
index c293ac14a..f0dd0a5c5 100644
--- a/lib/puppet/util/autoload.rb
+++ b/lib/puppet/util/autoload.rb
@@ -131,11 +131,23 @@ class Puppet::Util::Autoload
     # 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)