From 23830504dfeb48b2d162e44b84b6f9dfa97e4e7e Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Thu, 22 Jul 2010 20:36:24 +0200 Subject: 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 Second-author: Markus Roberts --- lib/puppet/parser/compiler.rb | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'lib/puppet/parser/compiler.rb') diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb index a901c0dd6..760d5a75a 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] -- cgit From 1d494a3104e9794cc09ba27c701ced68a74fa398 Mon Sep 17 00:00:00 2001 From: Markus Roberts Date: Fri, 23 Jul 2010 08:47:48 -0700 Subject: Tweak to fix for #4302--dangling ref to known_resource_types Since we were clearing the thread variable containing the compiler's reference to it's environment's known resource types at the start of each compile the reference remaining at the end of a compilation could never be used and was thus just garbage that we were arbitrarily retaining. This patch moves the clearing of the thread var to the _end_ of compilation so that it's always nil except in the middle of a compile. This raises an interesting question; should the ref just live on the compiler object and we could dispense with the thread-var? It might require things that now only know about the environment to need a ref to the compiler and introduce other thread issues (e.g. we might just end up needing a :current_compiler thread variable, for no net gain in simplicity). --- lib/puppet/parser/compiler.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'lib/puppet/parser/compiler.rb') diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb index 760d5a75a..61bb13cb6 100644 --- a/lib/puppet/parser/compiler.rb +++ b/lib/puppet/parser/compiler.rb @@ -15,16 +15,15 @@ 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] raise Puppet::Error, "#{detail} on node #{node.name}" - end + ensure + # 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 + end attr_reader :node, :facts, :collections, :catalog, :node_scope, :resources, :relationships -- cgit