adding a new 2nd pass to the optimizer for hoisting invariant aliases.

this fixes a few important bugs and should achieve better performance in a 
number of complex use cases.
added a number of debug options to the test runner.

CHANGES

@@ -2,10 +2,15 @@
 found a bug in the optimizer due to some half-baked techniques in the IfNode.
 the best fix is to disable this type of optimization (which I've done). the
 code emitted is now functional, but not optimal.  I've added an exermintal
-optimization type to -03 which uses a second pass on function blocks to hoist
-invariant aliases.  although this works in terms of a prototype, the initial
-code is iterative rather than recursive, so this is a problem for completeness.
-added a covering test case for the specific bug.
+optimization type to -03 which uses a second pass over the optimized ast to 
+hoist branch and look invariant aliases. this required a number of sensitive
+changes to the 'scoping' code, especially with loops. overally, i think the
+extra pass over the optimized ast keeps the code a little more simple, but it
+still feels like it should not be necessary.
+
+i added a number of new tests to check these optimizations. some of these are
+currently expected to fail. added a bunch more debugging code to the testing 
+front end script to make it easier to diagnose problems as they show up.
 
 0.6.16 - 2008-03-18
 changed the placeholder grammar and added a test case for a particularly

Makefile

@@ -25,6 +25,8 @@ no_whitespace_tests: clean_tests parser
 	$(CRUNNER) -O1 --test-input tests/input/search_list_data.pye -qt tests/*txt tests/*tmpl
 	$(COMPILER) -O2 tests/*txt tests/*tmpl
 	$(CRUNNER) -O2 --test-input tests/input/search_list_data.pye -qt tests/*txt tests/*tmpl
+	$(COMPILER) -O3 tests/*txt tests/*tmpl
+	$(CRUNNER) -O3 --test-input tests/input/search_list_data.pye -qt tests/*txt tests/*tmpl
 
 .PHONY : whitespace_tests
 whitespace_tests: clean_tests parser
@@ -34,11 +36,6 @@ whitespace_tests: clean_tests parser
 	$(CRUNNER) -O1 --preserve-optional-whitespace --test-input tests/input/search_list_data.pye --test-output output-preserve-whitespace -qt tests/*txt tests/*tmpl
 	$(COMPILER) -O2 --preserve-optional-whitespace tests/*txt tests/*tmpl
 	$(CRUNNER) -O2 --preserve-optional-whitespace --test-input tests/input/search_list_data.pye --test-output output-preserve-whitespace -qt tests/*txt tests/*tmpl
-
-.PHONY : opt_tests
-opt_tests: clean_tests parser
-	$(COMPILER) -O3 tests/*txt tests/*tmpl
-	$(CRUNNER) -O3 --test-input tests/input/search_list_data.pye -qt tests/*txt tests/*tmpl
 	$(COMPILER) -O3 --preserve-optional-whitespace tests/*txt tests/*tmpl
 	$(CRUNNER) -O3 --preserve-optional-whitespace --test-input tests/input/search_list_data.pye --test-output output-preserve-whitespace -qt tests/*txt tests/*tmpl
 

TODO

@@ -30,7 +30,7 @@ Functionality:
      should be more consistent. right now you must have it in the def, but you
      can't have it when you do the calling.
  * add #encoding directive
- * add a build time/version attribute?
+ * add a build time/version/runtime_version attribute?
  * xhtml-oriented front end
    * emulate virtual odd/even var in loops (partially implemented Repeater)
  * Attribute Language - (like TAL and Kid)
@@ -93,6 +93,9 @@ Variant 1.1:
 Variant 2:
         write(u"""<td>%s</td>\n""" % column)
 
+Varian 2.1: (UNTESTED)
+        write(u"""<td>""" + "%s" % column + u"""</td>\n""")
+
    * optimization of this loop is dependent on the python version (2.4 vs 2.5)
    * Variant 2 is universally the slowest
 

scripts/crunner.py

@@ -19,6 +19,28 @@
 import spitfire.runtime.runner
 
 
+# this class let's me check if placeholder caching is working properly by
+# tracking the number of accesses for a single key
+class ResolveCounter(object):
+  def _get_item(self, key):
+    if key in self.__dict__:
+      self.__dict__[key] += 1
+    else:
+      self.__dict__[key] = 1
+
+    return '%s%s' % (key, self.__dict__[key])
+
+  def __getitem__(self, key):
+    if not key.startswith('resolve'):
+      raise KeyError(key)
+    return self._get_item(key)
+
+  def __getattr__(self, key):
+    if not key.startswith('resolve'):
+      raise AttributeError(key)
+    return self._get_item(key)
+
+
 def print_tree_walk(node, indent=0):
   if indent > 5:
     raise 'error'
@@ -45,30 +67,34 @@ def print_output(*args):
     print_output("compile", filename)
     compiler = spitfire.compiler.util.Compiler(analyzer_options=opt, **args)
     if not options.quiet:
-      print "parse_root walk"
       parse_root = spitfire.compiler.util.parse_file(filename, options.xhtml)
-      #print_tree_walk(parse_root)
-      print_tree(parse_root)
+      if 'parse_tree' in options.debug_flags:
+        print "parse_root walk"
+        print_tree(parse_root)
 
     if not options.quiet:
-      print "ast_root walk"
       ast_root = spitfire.compiler.analyzer.SemanticAnalyzer(
         classname, parse_root, opt, compiler).get_ast()
-      print_tree(ast_root)
+      if 'ast' in options.debug_flags:
+        print "ast_root walk"
+        print_tree(ast_root)
 
     if not options.quiet:
-      print "optimized ast_root walk"
       spitfire.compiler.optimizer.OptimizationAnalyzer(
         ast_root, opt, compiler).optimize_ast()
-      print_tree(ast_root)
-
+      spitfire.compiler.optimizer.FinalPassAnalyzer(
+        ast_root, opt, compiler).optimize_ast()
+      if 'optimized_ast' in options.debug_flags:
+        print "optimized ast_root walk"
+        print_tree(ast_root)
+
     if not options.quiet:
-      print "src_code"
       src_code = spitfire.compiler.codegen.CodeGenerator(
         ast_root, opt).get_code()
-      #src_code = spitfire.compiler.util.compile_file(filename, options=opt)
-      for i, line in enumerate(src_code.split('\n')):
-        print '% 3s' % (i + 1), line
+      if 'codegen' in options.debug_flags:
+        print "src_code"
+        for i, line in enumerate(src_code.split('\n')):
+          print '% 3s' % (i + 1), line
   except Exception, e:
     print >> sys.stderr, "FAILED:", classname, e
     raise
@@ -78,7 +104,10 @@ def print_output(*args):
 
     if options.test_input:
       search_list = [
-        spitfire.runtime.runner.load_search_list(options.test_input)]
+        spitfire.runtime.runner.load_search_list(options.test_input),
+        ResolveCounter(),
+        {'nested_resolver':ResolveCounter()},
+        ]
     else:
       search_list = []
 
@@ -151,7 +180,16 @@ def print_output(*args):
   op.add_option('-O', dest='optimizer_level', type='int', default=0)
   op.add_option('--disable-filters', dest='enable_filters',
                 action='store_false', default=True)
+  help = ', '.join([x.replace('_', '-')
+                    for x in dir(analyzer.AnalyzerOptions())
+                    if not x.startswith('__')])
+  op.add_option('-X', dest='optimizer_flags', action='append', default=[],
+                help=(analyzer.AnalyzerOptions.get_help()))
+  op.add_option('-D', dest='debug_flags', action='store',
+                default='ast,optimized_ast,codegen',
+                help='parse_tree, ast, optimized_ast, codegen'
+                )
   (options, args) = op.parse_args()
-
+  setattr(options, 'debug_flags', getattr(options, 'debug_flags').split(','))
   for filename in args:
     process_file(filename, options)

scripts/spitfire-compile

@@ -94,6 +94,8 @@ if __name__ == '__main__':
                 type="str", nargs=1,
                 help='file to use as the message catalogue')
   op.add_option('--locale', default='')
+  op.add_option('-X', dest='optimizer_flags', action='append', default=[],
+                help=analyzer.AnalyzerOptions.get_help())
 
   (options, args) = op.parse_args()
 

spitfire/compiler/analyzer.py

@@ -38,18 +38,31 @@ def __init__(self, **kargs):
     # once a placeholder is resolved in a given scope, cache it in a local
     # reference for faster subsequent retrieval
     self.cache_resolved_placeholders = False
+
+    # when adding an alias, detect if the alias is loop invariant and hoist
+    # right there on the spot.  this has probably been superceded by
+    # hoist_loop_invariant_aliases, but does have the advantage of not needing
+    # another pass over the tree
+    self.inline_hoist_loop_invariant_aliases = False
 
     # if an alias has been generated in a conditional scope and it is also
     # defined in the parent scope, hoist it above the conditional. this
     # requires a two-pass optimization on functions, which adds time and
     # complexity
     self.hoist_conditional_aliases = False
+    self.hoist_loop_invariant_aliases = False
 
     self.enable_psyco = False
     self.__dict__.update(kargs)
 
   def update(self, **kargs):
     self.__dict__.update(kargs)
+
+  @classmethod
+  def get_help(cls):
+    return ', '.join(['[no-]' + name.replace('_', '-')
+                    for name, value in vars(cls()).iteritems()
+                    if not name.startswith('__') and type(value) == bool])
 
 default_options = AnalyzerOptions()
 o1_options = copy.copy(default_options)
@@ -59,9 +72,12 @@ def update(self, **kargs):
 o2_options.alias_invariants = True
 o2_options.directly_access_defined_variables = True
 o2_options.cache_resolved_placeholders = True
+o2_options.inline_hoist_loop_invariant_aliases = True
 
 o3_options = copy.copy(o2_options)
+o3_options.inline_hoist_loop_invariant_aliases = False
 o3_options.hoist_conditional_aliases = True
+o3_options.hoist_loop_invariant_aliases = True
 o3_options.enable_psyco = True
 
 optimizer_map = {

spitfire/compiler/ast.py

@@ -223,7 +223,8 @@ def __init__(self, target_list=None, expression_list=None):
       self.expression_list = expression_list
     else:
       self.expression_list = ExpressionListNode()
-
+    self.scope = Scope('ForNode')
+
   def __str__(self):
     return ('%s target_list:%s expr_list:%s' %
             (self.__class__.__name__, self.target_list, self.expression_list))
@@ -246,8 +247,7 @@ def __init__(self, *pargs, **kargs):
         CallFunctionNode(GetAttrNode(IdentifierNode('_buffer'), 'getvalue'))),
       ])
     self.parameter_list = ParameterListNode()
-    self.scope = Scope()
-    self.hoisted_aliases = []
+    self.scope = Scope('Function')
 
   def append(self, node):
     self.child_nodes.insert(-1, node)
@@ -307,7 +307,7 @@ def __init__(self, test_expression=None):
     ASTNode.__init__(self)
     self.test_expression = test_expression
     self.else_ = ElseNode(self)
-    self.scope = Scope()
+    self.scope = Scope('If')
 
   def replace(self, node, replacement_node):
     if self.test_expression is node:
@@ -323,7 +323,7 @@ class ElseNode(ASTNode):
   def __init__(self, parent=None):
     ASTNode.__init__(self)
     self.parent = parent
-    self.scope = Scope()
+    self.scope = Scope('Else')
 
 class ImplementsNode(ASTNode):
   pass
@@ -537,10 +537,14 @@ def replace(self, node, replacement_node):
 # templates and shoe-horned that into python
 class Scope(object):
   def __init__(self, name=None):
-    self.name = name
+    if name:
+      self.name = name
+    else:
+      self.name = hex(id(self))
     self.local_identifiers = []
     self.aliased_expression_map = {}
     self.alias_name_set = set()
+    self.hoisted_aliases = []
 
   def __str__(self):
     return "<Scope %(name)s> %(alias_name_set)s" % vars(self)