Many, many, many performance improvements in the compiler (I hope). I did not change functionality anywhere, but I did some profiling and significantly reduced the runtime of many methods, and especially focused on some key methods that run many times.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@1739 980ebf18-57e1-0310-9a29-db15c13687c0

10 files changed, 181 insertions, 176 deletions

diff --git a/lib/puppet/parser/ast.rb b/lib/puppet/parser/ast.rb
index 6f9c9492c..ccd6d216a 100644
--- a/lib/puppet/parser/ast.rb
+++ b/lib/puppet/parser/ast.rb
@@ -43,6 +43,15 @@ class Puppet::Parser::AST
         return @@midline
     end
 
+    # Does this ast object set something?  If so, it gets evaluated first.
+    def self.settor?
+        if defined? @settor
+            @settor
+        else
+            false
+        end
+    end
+
     # Evaluate the current object.  Basically just iterates across all
     # of the contained children and evaluates them in turn, returning a
     # list of all of the collected values, rejecting nil values
@@ -72,8 +81,23 @@ class Puppet::Parser::AST
     # it can enable you to catch the error where it happens, rather than
     # much higher up the stack.
     def safeevaluate(*args)
-        exceptwrap do
-            self.evaluate(*args)
+        # We duplicate code here, rather than using exceptwrap, because this
+        # is called so many times during parsing.
+        #exceptwrap do
+        #    self.evaluate(*args)
+        #end
+        begin
+            return self.evaluate(*args)
+        rescue Puppet::Error => detail
+            raise adderrorcontext(detail)
+        rescue => detail
+            message = options[:message] || "%s failed with error %s: %s" %
+                    [self.class, detail.class, detail.to_s]
+
+            error = options[:type].new(message)
+            # We can't use self.fail here because it always expects strings,
+            # not exceptions.
+            raise adderrorcontext(error, detail)
         end
     end
 
@@ -96,8 +120,6 @@ class Puppet::Parser::AST
     end
     #---------------------------------------------------------------
     # Now autoload everything.
-    # XXX We can't do this, because it causes multiple loads of some
-    # things.
     @autoloader = Puppet::Autoload.new(self,
         "puppet/parser/ast"
     )
diff --git a/lib/puppet/parser/ast/astarray.rb b/lib/puppet/parser/ast/astarray.rb
index fb0a3f671..a0bd5bf89 100644
--- a/lib/puppet/parser/ast/astarray.rb
+++ b/lib/puppet/parser/ast/astarray.rb
@@ -25,10 +25,6 @@ class Puppet::Parser::AST
                 # This is such a stupid hack.  I've no real idea how to make a
                 # "real" declarative language, so I hack it so it looks like
                 # one, yay.
-                setlist = [
-                    AST::VarDef, AST::ResourceDefaults, AST::Function
-                ]
-
                 settors = []
                 others = []
 
@@ -40,34 +36,30 @@ class Puppet::Parser::AST
                 @children.each { |child|
                     if child.instance_of?(AST::ASTArray)
                         child.each do |ac|
-                            items << ac
+                            if ac.class.settor?
+                                settors << ac
+                            else
+                                others << ac
+                            end
                         end
                     else
-                        items << child
-                    end
-                }
-
-                # Now sort them all according to the type of action
-                items.each { |child|
-                    if setlist.include?(child.class)
-                        settors << child
-                    else
-                        others << child
+                        if child.class.settor?
+                            settors << child
+                        else
+                            others << child
+                        end
                     end
                 }
                 rets = [settors, others].flatten.collect { |child|
                     child.safeevaluate(:scope => scope)
                 }
+                return rets.reject { |o| o.nil? }
             else
                 # If we're not declarative, just do everything in order.
-                rets = @children.collect { |item|
+                return @children.collect { |item|
                     item.safeevaluate(:scope => scope)
-                }
+                }.reject { |o| o.nil? }
             end
-
-            rets = rets.reject { |obj| obj.nil? }
-
-            return rets
         end
 
         def push(*ary)
diff --git a/lib/puppet/parser/ast/component.rb b/lib/puppet/parser/ast/component.rb
index 312b11d99..f67d53b4b 100644
--- a/lib/puppet/parser/ast/component.rb
+++ b/lib/puppet/parser/ast/component.rb
@@ -96,17 +96,29 @@ class Puppet::Parser::AST
             @parentclass = nil
             super
 
-            # Deal with metaparams in the argument list.
+            # Convert the arguments to a hash for ease of later use.
             if @arguments
-                @arguments.each do |arg, defvalue|
-                    next unless Puppet::Type.metaparamclass(arg)
-                    if defvalue
-                        warnonce "%s is a metaparam; this value will inherit to all contained elements" % arg
-                    else
-                        raise Puppet::ParseError,
-                            "%s is a metaparameter; please choose another name" %
-                            name
-                    end
+                unless @arguments.is_a? Array
+                    @arguments = [@arguments]
+                end
+                oldargs = @arguments
+                @arguments = {}
+                oldargs.each do |arg, val|
+                    @arguments[arg] = val
+                end
+            else
+                @arguments = {}
+            end
+
+            # Deal with metaparams in the argument list.
+            @arguments.each do |arg, defvalue|
+                next unless Puppet::Type.metaparamclass(arg)
+                if defvalue
+                    warnonce "%s is a metaparam; this value will inherit to all contained elements" % arg
+                else
+                    raise Puppet::ParseError,
+                        "%s is a metaparameter; please choose another name" %
+                        name
                 end
             end
         end
@@ -120,7 +132,7 @@ class Puppet::Parser::AST
         # Set our parent class, with a little check to avoid some potential
         # weirdness.
         def parentclass=(name)
-            if name == @type
+            if name == self.type
                 parsefail "Parent classes must have dissimilar names"
             end
 
@@ -171,35 +183,23 @@ class Puppet::Parser::AST
         # Check whether a given argument is valid.  Searches up through
         # any parent classes that might exist.
         def validattr?(param)
-            return true if Puppet::Type.metaparam?(param)
-
             param = param.to_s
-            found = false
-            unless @arguments.is_a? Array
-                @arguments = [@arguments]
-            end
 
-            found = @arguments.detect { |arg|
-                if arg.is_a? Array
-                    arg[0] == param
-                else
-                    arg == param
-                end
-            }
-
-            if found
+            if @arguments.include?(param)
                 # It's a valid arg for us
                 return true
-            elsif defined? @parentclass and @parentclass
-                # Else, check any existing parent
-                if parent = @scope.lookuptype(@parentclass) and parent != []
-                    return parent.validarg?(param)
-                elsif builtin = Puppet::Type.type(@parentclass)
-                    return builtin.validattr?(param)
-                else
-                    raise Puppet::Error, "Could not find parent class %s" %
-                        @parentclass
-                end
+#            elsif defined? @parentclass and @parentclass
+#                # Else, check any existing parent
+#                if parent = @scope.lookuptype(@parentclass) and parent != []
+#                    return parent.validarg?(param)
+#                elsif builtin = Puppet::Type.type(@parentclass)
+#                    return builtin.validattr?(param)
+#                else
+#                    raise Puppet::Error, "Could not find parent class %s" %
+#                        @parentclass
+#                end
+            elsif Puppet::Type.metaparam?(param)
+                return true
             else
                 # Or just return false
                 return false
diff --git a/lib/puppet/parser/ast/function.rb b/lib/puppet/parser/ast/function.rb
index f67531ae3..6729431b7 100644
--- a/lib/puppet/parser/ast/function.rb
+++ b/lib/puppet/parser/ast/function.rb
@@ -3,6 +3,8 @@ class Puppet::Parser::AST
     class Function < AST::Branch
         attr_accessor :name, :arguments
 
+        @settor = true
+
         def evaluate(hash)
             # We don't need to evaluate the name, because it's plaintext
 
diff --git a/lib/puppet/parser/ast/resourceparam.rb b/lib/puppet/parser/ast/resourceparam.rb
index 248e91b32..317820a3c 100644
--- a/lib/puppet/parser/ast/resourceparam.rb
+++ b/lib/puppet/parser/ast/resourceparam.rb
@@ -12,17 +12,12 @@ class Puppet::Parser::AST
         # Return the parameter and the value.
         def evaluate(hash)
             scope = hash[:scope]
-            param = @param
-            value = @value.safeevaluate(:scope => scope)
 
-            args = {:name => param, :value => value, :source => scope.source}
-            [:line, :file].each do |p|
-                if v = self.send(p)
-                    args[p] = v
-                end
-            end
-
-            return Puppet::Parser::Resource::Param.new(args)
+            return Puppet::Parser::Resource::Param.new(
+                :name => @param,
+                :value => @value.safeevaluate(:scope => scope),
+                :source => scope.source, :line => self.line, :file => self.file
+            )
         end
 
         def tree(indent = 0)
diff --git a/lib/puppet/parser/ast/vardef.rb b/lib/puppet/parser/ast/vardef.rb
index 30eef204a..145700e27 100644
--- a/lib/puppet/parser/ast/vardef.rb
+++ b/lib/puppet/parser/ast/vardef.rb
@@ -3,6 +3,8 @@ class Puppet::Parser::AST
     class VarDef < AST::Branch
         attr_accessor :name, :value
 
+        @settor = true
+
         # Look up our name and value, and store them appropriately.  The
         # lexer strips off the syntax stuff like '$'.
         def evaluate(hash)
diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb
index 5d09eaa9b..af5d10501 100644
--- a/lib/puppet/parser/resource.rb
+++ b/lib/puppet/parser/resource.rb
@@ -85,25 +85,42 @@ class Puppet::Parser::Resource
 
     # Retrieve the associated definition and evaluate it.
     def evaluate
-        if builtin?
+        if klass = @ref.definedtype
+            finish()
+            scope.deleteresource(self)
+            return klass.evaluate(:scope => scope,
+                                  :type => self.type,
+                                  :name => self.title,
+                                  :arguments => self.to_hash,
+                                  :scope => self.scope,
+                                  :exported => self.exported
+            )
+        elsif builtin?
             devfail "Cannot evaluate a builtin type"
-        end
-
-        unless klass = scope.finddefine(self.type)
+        else
             self.fail "Cannot find definition %s" % self.type
         end
 
-        finish()
-
-        scope.deleteresource(self)
 
-        return klass.evaluate(:scope => scope,
-                              :type => self.type,
-                              :name => self.title,
-                              :arguments => self.to_hash,
-                              :scope => self.scope,
-                              :exported => self.exported
-        )
+#        if builtin?
+#            devfail "Cannot evaluate a builtin type"
+#        end
+#
+#        unless klass = scope.finddefine(self.type)
+#            self.fail "Cannot find definition %s" % self.type
+#        end
+#
+#        finish()
+#
+#        scope.deleteresource(self)
+#
+#        return klass.evaluate(:scope => scope,
+#                              :type => self.type,
+#                              :name => self.title,
+#                              :arguments => self.to_hash,
+#                              :scope => self.scope,
+#                              :exported => self.exported
+#        )
     ensure
         @evaluated = true
     end
@@ -138,8 +155,7 @@ class Puppet::Parser::Resource
 
         # Collect the options necessary to make the reference.
         refopts = [:type, :title].inject({}) do |hash, param|
-            hash[param] = options[param] ||
-                devfail("%s must be passed to Resources" % param)
+            hash[param] = options[param]
             options.delete(param)
             hash
         end
@@ -175,22 +191,25 @@ class Puppet::Parser::Resource
         end
     end
 
+    # This *significantly* reduces the number of calls to Puppet.[].
+    def paramcheck?
+        unless defined? @@paramcheck
+            @@paramcheck = Puppet[:paramcheck]
+        end
+        @@paramcheck
+    end
+
     # Verify that all passed parameters are valid.  This throws an error if there's
     # a problem, so we don't have to worry about the return value.
     def paramcheck(param)
-        # This defaults to true
-        unless Puppet[:paramcheck]
-            return
-        end
-
-        return if param == "name" or param == "title" # always allow these
-
-        # FIXME We might need to do more here eventually.  Metaparams
-        # behave strangely on containers.
-        return if Puppet::Type.metaparam?(param)
-
-        # Now make sure it's a valid argument to our class.
-        unless @ref.typeclass.validattr?(param)
+        # Now make sure it's a valid argument to our class.  These checks
+        # are organized in order of commonhood -- most types, it's a valid argument
+        # and paramcheck is enabled.
+        if @ref.typeclass.validattr?(param)
+            true
+        elsif (param == "name" or param == "title") # always allow these
+            true
+        elsif paramcheck?
             self.fail Puppet::ParseError, "Invalid parameter '%s' for type '%s'" %
                     [param.inspect, @ref.type]
         end
diff --git a/lib/puppet/parser/resource/param.rb b/lib/puppet/parser/resource/param.rb
index 259e935d1..600e44781 100644
--- a/lib/puppet/parser/resource/param.rb
+++ b/lib/puppet/parser/resource/param.rb
@@ -1,13 +1,14 @@
 # The parameters we stick in Resources.
 class Puppet::Parser::Resource::Param
     attr_accessor :name, :value, :source, :line, :file
+    include Puppet::Util
     include Puppet::Util::Errors
     include Puppet::Util::MethodHelper
 
     def initialize(hash)
         set_options(hash)
         requiredopts(:name, :value, :source)
-        @name = @name.intern if @name.is_a?(String)
+        @name = symbolize(@name)
     end
 
     def inspect
diff --git a/lib/puppet/parser/resource/reference.rb b/lib/puppet/parser/resource/reference.rb
index 2210b71c2..f71b3719f 100644
--- a/lib/puppet/parser/resource/reference.rb
+++ b/lib/puppet/parser/resource/reference.rb
@@ -49,7 +49,10 @@ class Puppet::Parser::Resource::Reference
     end
 
     def to_s
-        "%s[%s]" % [type, title]
+        unless defined? @namestring
+            @namestring = "%s[%s]" % [type, title]
+        end
+        @namestring
     end
 
     def typeclass
diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb
index 9b59ebd64..02fd72b0d 100644
--- a/lib/puppet/parser/scope.rb
+++ b/lib/puppet/parser/scope.rb
@@ -24,7 +24,7 @@ class Puppet::Parser::Scope
     include Puppet::Util::Errors
     attr_accessor :parent, :level, :interp, :source, :host
     attr_accessor :name, :type, :topscope, :base, :keyword, :namespace
-    attr_accessor :top, :context, :translated, :exported
+    attr_accessor :top, :translated, :exported
 
     # Whether we behave declaratively.  Note that it's a class variable,
     # so all scopes behave the same.
@@ -83,13 +83,11 @@ class Puppet::Parser::Scope
 
     # Verify that the given object isn't defined elsewhere.
     def chkobjectclosure(obj)
-        if @definedtable.include?(obj.ref)
+        if exobj = @definedtable[obj.ref]
             typeklass = Puppet::Type.type(obj.type)
             if typeklass and ! typeklass.isomorphic?
                 Puppet.info "Allowing duplicate %s" % type
             else
-                exobj = @definedtable[obj.ref]
-
                 # Either it's a defined type, which are never
                 # isomorphic, or it's a non-isomorphic type.
                 msg = "Duplicate definition: %s is already defined" % obj.ref
@@ -238,8 +236,7 @@ class Puppet::Parser::Scope
             @interp = @parent.interp
             @source = hash[:source] || @parent.source
             @topscope = @parent.topscope
-            @context = @parent.context
-            @inside = @parent.inside
+            #@inside = @parent.inside # Used for definition inheritance
             @host = @parent.host
             @type ||= @parent.type
         end
@@ -247,20 +244,14 @@ class Puppet::Parser::Scope
         # Our child scopes and objects
         @children = []
 
-        # The symbol table for this scope
-        @symtable = Hash.new(nil)
+        # The symbol table for this scope.  This is where we store variables.
+        @symtable = {}
 
         # All of the defaults set for types.  It's a hash of hashes,
         # with the first key being the type, then the second key being
         # the parameter.
         @defaultstable = Hash.new { |dhash,type|
-            dhash[type] = Hash.new(nil)
-        }
-
-        # Map the names to the tables.
-        @map = {
-            "variable" => @symtable,
-            "defaults" => @defaultstable
+            dhash[type] = {}
         }
 
         unless @interp
@@ -288,7 +279,7 @@ class Puppet::Parser::Scope
 
         # The table for storing class singletons.  This will only actually
         # be used by top scopes and node scopes.
-        @classtable = Hash.new(nil)
+        @classtable = {}
 
         self.class.declarative = declarative
 
@@ -315,7 +306,6 @@ class Puppet::Parser::Scope
         # but they each refer back to the scope that created them.
         @collecttable = []
 
-        @context = nil
         @topscope = self
         @type = "puppet"
         @name = "top"
@@ -369,15 +359,15 @@ class Puppet::Parser::Scope
     # Look up a variable.  The simplest value search we do.  Default to returning
     # an empty string for missing values, but support returning a constant.
     def lookupvar(name, usestring = true)
-        value = lookup("variable", name)
-        if value == :undefined
-            if usestring
-                return ""
-            else
-                return :undefined
-            end
+        # We can't use "if @symtable[name]" here because the value might be false
+        if @symtable.include?(name)
+            return @symtable[name]
+        elsif self.parent 
+            return @parent.lookupvar(name, usestring)
+        elsif usestring
+            return ""
         else
-            return value
+            return :undefined
         end
     end
 
@@ -395,7 +385,8 @@ class Puppet::Parser::Scope
 
     # Return the list of remaining overrides.
     def overrides
-        @overridetable.collect { |name, overs| overs }.flatten
+        #@overridetable.collect { |name, overs| overs }.flatten
+        @overridetable.values.flatten
     end
 
     def resources
@@ -492,29 +483,36 @@ class Puppet::Parser::Scope
     def setvar(name,value)
         #Puppet.debug "Setting %s to '%s' at level %s" %
         #    [name.inspect,value,self.level]
-        if @@declarative and @symtable.include?(name)
-            raise Puppet::ParseError, "Cannot reassign variable %s" % name
-        else
-            if @symtable.include?(name)
+        if @symtable.include?(name)
+            if @@declarative
+                raise Puppet::ParseError, "Cannot reassign variable %s" % name
+            else
                 Puppet.warning "Reassigning %s to %s" % [name,value]
             end
-            @symtable[name] = value
         end
+        @symtable[name] = value
     end
 
     # Return an interpolated string.
     def strinterp(string)
-        newstring = string.gsub(/\\\$|\$\{(\w+)\}|\$(\w+)/) do |value|
-            # If it matches the backslash, then just retun the dollar sign.
-            if value == '\\$'
-                '$'
-            else # look the variable up
-                var = $1 || $2
-                lookupvar($1 || $2)
+        # Most strings won't have variables in them.
+        if string =~ /\$/
+            string = string.gsub(/\\\$|\$\{(\w+)\}|\$(\w+)/) do |value|
+                # If it matches the backslash, then just retun the dollar sign.
+                if value == '\\$'
+                    '$'
+                else # look the variable up
+                    lookupvar($1 || $2)
+                end
             end
         end
 
-        return newstring.gsub(/\\t/, "\t").gsub(/\\n/, "\n").gsub(/\\s/, "\s")
+        # And most won't have whitespace replacements.
+        if string =~ /\\/
+            return string.gsub(/\\t/, "\t").gsub(/\\n/, "\n").gsub(/\\s/, "\s")
+        else
+            return string
+        end
     end
 
     # Add a tag to our current list.  These tags will be added to all
@@ -573,20 +571,19 @@ class Puppet::Parser::Scope
     def translate
         ret = @children.collect do |child|
             case child
-            when self.class
-                child.translate
             when Puppet::Parser::Resource
                 child.to_trans
+            when self.class
+                child.translate
             else
                 devfail "Got %s for translation" % child.class
             end
         end.reject { |o| o.nil? }
         bucket = Puppet::TransBucket.new ret
-        unless self.type
-            devfail "A Scope with no type"
-        end
-        if @type == ""
-            bucket.type = "main"
+
+        case self.type
+        when "": bucket.type = "main"
+        when nil: devfail "A Scope with no type"
         else
             bucket.type = @type
         end
@@ -615,34 +612,6 @@ class Puppet::Parser::Scope
             return ary
         end
     end
-
-    protected
-
-    # This method abstracts recursive searching.  It accepts the type
-    # of search being done and then either a literal key to search for or
-    # a Proc instance to do the searching.
-    def lookup(type,sub, usecontext = false)
-        table = @map[type]
-        if table.nil?
-            self.fail "Could not retrieve %s table at level %s" %
-                    [type,self.level]
-        end
-
-        if sub.is_a?(Proc) and obj = sub.call(table)
-            return obj
-        elsif table.include?(sub)
-            return table[sub]
-        elsif ! @parent.nil?
-            # Context is used for retricting overrides.
-            if usecontext and self.context != @parent.context
-                return :undefined
-            else
-                return @parent.lookup(type,sub, usecontext)
-            end
-        else
-            return :undefined
-        end
-    end
 end
 
 # $Id$