[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. 2.6.1rc1-141-gcdb2b90

[Markus Roberts]

The following commit has been merged in the upstream branch:
commit 23830504dfeb48b2d162e44b84b6f9dfa97e4e7e
Author: Brice Figureau <brice-puppet at daysofwonder.com>
Date:   Thu Jul 22 20:36:24 2010 +0200

    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 at daysofwonder.com>
    Second-author: Markus Roberts <markus at puppetlabs.com>

diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
index 762599c..3f67474 100644
--- a/lib/puppet/node/environment.rb
+++ b/lib/puppet/node/environment.rb
@@ -1,4 +1,5 @@
 require 'puppet/util/cacher'
+require 'monitor'
 
 # Just define it, so this class has fewer load dependencies.
 class Puppet::Node
@@ -67,14 +68,23 @@ class Puppet::Node::Environment
 
   def initialize(name)
     @name = name
+    extend MonitorMixin
   end
 
   def known_resource_types
-    if @known_resource_types.nil? or @known_resource_types.stale?
-      @known_resource_types = Puppet::Resource::TypeCollection.new(self)
-      @known_resource_types.perform_initial_import
-    end
-    @known_resource_types
+    # This makes use of short circuit evaluation to get the right thread-safe
+    # per environment semantics with an efficient most common cases; we almost
+    # always just return our thread's known-resource types.  Only at the start
+    # of a compilation (after our thread var has been set to nil) or when the
+    # environment has changed do we delve deeper. 
+    Thread.current[:known_resource_types] = nil if (krt = Thread.current[:known_resource_types]) && krt.environment != self
+    Thread.current[:known_resource_types] ||= synchronize {
+      if @known_resource_types.nil? or @known_resource_types.stale?
+        @known_resource_types = Puppet::Resource::TypeCollection.new(self)
+        @known_resource_types.perform_initial_import
+      end
+      @known_resource_types
+    }
   end
 
   def module(name)
diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
index a901c0d..760d5a7 100644
--- a/lib/puppet/parser/compiler.rb
+++ b/lib/puppet/parser/compiler.rb
@@ -15,6 +15,11 @@ class Puppet::Parser::Compiler
   include Puppet::Resource::TypeCollectionHelper
 
   def self.compile(node)
+    # At the start of a new compile we don't assume anything about 
+    # known_resouce_types; we'll get these from the environment and
+    # cache them in a thread variable for the duration of the 
+    # compilation.
+    Thread.current[:known_resource_types] = nil
     new(node).compile.to_resource
   rescue => detail
     puts detail.backtrace if Puppet[:trace]
diff --git a/spec/unit/node/environment_spec.rb b/spec/unit/node/environment_spec.rb
index b400865..6edcce5 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|

-- 
Puppet packaging for Debian