Please review pull request #753: Significantly improve compilation performance when using modules opened by (nicklewis)
Description:
When autoloading classes/defines, the typeloader constructs a set of
possible locations for the class, based on its name. Effectively, it
will look in the canonical locations corresponding to each namespace in
the fully-qualified name. So for each namespace, it will ask the
environment for a Puppet::Module instance for that module, to ask it
which of the module's potentially manifests match the class it's looking
for. To answer that request, the environment instantiates a
Puppet::Module.
This amounts to potentially thousands of Puppet::Module instances being
created, because it does this many times (based on nesting of the class
name) per include/autoload/import. When Puppet::Module instances are
created, they parse and load their metadata.json file, in part to
validate their use. This implies that each compilation results in
metadata.json being parsed thousands of times, which is extremely slow
(and obviously provides no actual benefit).
Fortunately, the environment object already keeps a list of
Puppet::Module instances for every module in its modulepath. The fix
applied here is simply to change the environment such that it provides
modules by looking them up in its cached list, resulting in up to an
order of magnitude improvement in compilation time.
- Opened: Thu May 10 18:50:11 UTC 2012
- Based on: puppetlabs:2.7.x (7d3e09801ced1dc31276c6338de7af35f58774de)
- Requested merge: nicklewis:type-loader-performance (a7a532cb17b7bc60dac05e02b7db5531de7a4a8c)
Diff follows:
diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
index e09931d..a6fe39b 100644
--- a/lib/puppet/node/environment.rb
+++ b/lib/puppet/node/environment.rb
@@ -88,9 +88,7 @@ def known_resource_types
end
def module(name)
- mod = Puppet::Module.new(name, :environment => self)
- return nil unless mod.exist?
- mod
+ modules.find {|mod| mod.name == name}
end
def module_by_forge_name(forge_name)
diff --git a/spec/unit/node/environment_spec.rb b/spec/unit/node/environment_spec.rb
index 4b25599..28c35e5 100755
--- a/spec/unit/node/environment_spec.rb
+++ b/spec/unit/node/environment_spec.rb
@@ -161,12 +161,17 @@
end
it "should be able to return an individual module that exists in its module path" do
+ env.stubs(:modules).returns [Puppet::Module.new('one'), Puppet::Module.new('two'), Puppet::Module.new('three')]
- mod = mock 'module'
- Puppet::Module.expects(:new).with("one", :environment => env).returns mod
- mod.expects(:exist?).returns true
+ mod = env.module('one')
+ mod.should be_a(Puppet::Module)
+ mod.name.should == 'one'
+ end
+
+ it "should not return a module if the module doesn't exist" do
+ env.stubs(:modules).returns [Puppet::Module.new('one'), Puppet::Module.new('two'), Puppet::Module.new('three')]
- env.module("one").should equal(mod)
+ env.module('four').should be_nil
end
it "should return nil if asked for a module that does not exist in its path" do
-- 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.
