I like this code. +1, unless someone can find a place where we're using the same thread to compile more than one catalog.
On Thu, Jul 22, 2010 at 11:37 AM, Brice Figureau < [email protected]> wrote: > In 2.6 we moved access to loaded type to the environment. > Each time the compiler was accessing the loaded types, we were checking > if the manifests did change. > This incurred a large performance decrease compared to 0.25. > This also introduced a race condition if manifests changed while a thread > was in the middle of a compilation operation. > > This tentative fix makes sure each thread will get access to the same > loaded types collection for his live, even if the manifests change. > Only new thread will notice the change and reload the manifests. > But as long as the manifest don't change, the thread will share the > same collection, so there is no duplicate in memory. > > Signed-off-by: Brice Figureau <[email protected]> > --- > lib/puppet/node/environment.rb | 6 +++++- > spec/unit/node/environment_spec.rb | 35 > ++++++++++++++++++++++++++++++++--- > 2 files changed, 37 insertions(+), 4 deletions(-) > > diff --git a/lib/puppet/node/environment.rb > b/lib/puppet/node/environment.rb > index 762599c..c2b8d28 100644 > --- a/lib/puppet/node/environment.rb > +++ b/lib/puppet/node/environment.rb > @@ -70,11 +70,15 @@ class Puppet::Node::Environment > end > > def known_resource_types > + return Thread.current[:known_resource_types] if > Thread.current[: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 > + > + Thread.current[:known_resource_types] = @known_resource_types > + Thread.current[:known_resource_types] > end > > def module(name) > 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| > -- > 1.6.6.1 > > -- > You received this message because you are subscribed to the Google Groups > "Puppet Developers" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]<puppet-dev%[email protected]> > . > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en. > > -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
