Fix #4302 - Compilation speed regression compared to 2.6

Each time the compiler was accessing the loaded types, we were checking if the
manifests had changed.  This incurred a large performance cost compared to 0.25
and introduced race conditions if manifests changed while a thread was in the
middle of a compilation.

This tentative fix, based on Brice's, makes sure each thread will get access to
the same loaded types collection for the durration of a compilation, even if
the manifests change.  We now only check for changed files at the start of a
compilation or if the environment changes, and we maintain a per environment
thread lock so that only one thread at a time can be reloading any particular
environment (and the need-check is done inside the synchronize block so that
only the first will actually load it).

As long as the manifests don't change, the threads will share the same collection,
so there is only duplication in memory for a brief window surrounding a change.

Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>
Second-author: Markus Roberts <markus@puppetlabs.com>

1 files changed, 32 insertions, 3 deletions

diff --git a/spec/unit/node/environment_spec.rb b/spec/unit/node/environment_spec.rb
index b400865a2..6edcce56c 100755
--- a/spec/unit/node/environment_spec.rb
+++ b/spec/unit/node/environment_spec.rb
@@ -53,6 +53,7 @@ describe Puppet::Node::Environment do
       @env = Puppet::Node::Environment.new("dev")
       @collection = Puppet::Resource::TypeCollection.new(@env)
       @collection.stubs(:perform_initial_import)
+      Thread.current[:known_resource_types] = nil
     end
 
     it "should create a resource type collection if none exists" do
@@ -71,13 +72,41 @@ describe Puppet::Node::Environment do
       @env.known_resource_types
     end
 
-    it "should create and return a new collection rather than returning a stale collection" do
-      @env.known_resource_types.expects(:stale?).returns true
+    it "should return the same collection even if stale if it's the same thread" do
+      Puppet::Resource::TypeCollection.stubs(:new).returns @collection
+      @env.known_resource_types.stubs(:stale?).returns true
 
-      Puppet::Resource::TypeCollection.expects(:new).returns @collection
+      @env.known_resource_types.should equal(@collection)
+    end
+
+    it "should return the current thread associated collection if there is one" do
+      Thread.current[:known_resource_types] = @collection
 
       @env.known_resource_types.should equal(@collection)
     end
+
+    it "should give to all threads the same collection if it didn't change" do
+      Puppet::Resource::TypeCollection.expects(:new).with(@env).returns @collection
+      @env.known_resource_types
+
+      t = Thread.new {
+        @env.known_resource_types.should equal(@collection)
+      }
+      t.join
+    end
+
+    it "should give to new threads a new collection if it isn't stale" do
+      Puppet::Resource::TypeCollection.expects(:new).with(@env).returns @collection
+      @env.known_resource_types.expects(:stale?).returns(true)
+
+      Puppet::Resource::TypeCollection.expects(:new).returns @collection
+
+      t = Thread.new {
+        @env.known_resource_types.should equal(@collection)
+      }
+      t.join
+    end
+
   end
 
   [:modulepath, :manifestdir].each do |setting|