[PPATCH] faster rendering through compiling ERB templates

"However, in order to get local assigns working, one needs to use instance variables and corresponding accessors for all the keys in local_assigns, as there is no way to pass local variables to an eval"

What about using method_missing?

I don't know what you mean by "What about using method_missing?". If you detail that comment, I might be able to comment it.

Ouch. It hurts to see a significant performance increase come at the cost of inconvenience and broken backward-compatibility. There must be a middle road.

How about passing local_assigns to the erb method?

# evalstr = "def #{erb_name}; #{erb.src}; end"
evalstr =<<end_eval
def #{erb_name}(local_assigns = nil)
  local_assigns.each { |key, value| eval %(#{key} = ObjectSpace._id2ref(value.id)) } if local_assigns
  #{erb.src}
end
end_eval

Also, evaluate_assigns is a poorer method name than evaluate_locals. How about evaluate_local_assigns.

The rest looks great. I look forward to testing it out..

More: have you seen ERB def_method, def_module, and def_class?

Andreas' suggestion of method_missing would be something like

# Assume the missing method is an attempt to reference one of our local_assigns
def method_missing(method, *args, &block)
  @local_assigns[method] or super
end

We'd have to create an anonymous Module or Class for each compiled template. But local_assigns can be different every time you render a template, so I don't think this will work as cleanly as evaluating the locals in the template's compiled method.

I am pretty sure that your suggestion will not work.

I dont' really understand the fuss about the restriction I mentioned. The difference to the orignal implementation is only minimal. It can only be detected when a template using 'a' => v1 includes another template using 'a' => v2 in local_assigns. I doubt this is the case in any app on the planet. If you are really paranoid about this, you can save the value of 'a' before rendering the sub template and restore that value after rendering the sub template. This will be faster then using missing_method.

local_assigns collision between templates is probably more common than you assume. Consider any app using recursive templates: rendering a master/detail hierarchy on a purchase order, showing nested comments in a forum, ..

Since an instance variable is set for each local assigns, they will also clash with instance variables set in your action.

Doing something like ERB#def_class could isolate the templates render without a big performance hit. Create a new anonymous class for each template with a result method that does the actual rendering. Instantiate the class with local_assigns for each render.

class CompiledTemplate
  def initialize(local_assigns = {})
    meta = class << self; self; end
    local_assigns.each do |key, value|
      instance_variable_set "@#{key}", value
      meta.send :attr_accessor, key
    end
  end

  # Anonymous subclasses override with a compiled ERB template.
  def result
  end
end

# in ActionView::Base
def compile_erb_template(template, file_name)
  cache_name = file_name || template
  unless klass = @@compiled_erb_template_classes[cache_name]
    klass = ERB.new(template, nil, '-').def_class(CompiledTemplate)
    @@compiled_erb_template_classes[cache_name] = klass
    @@loaded_templates[cache_name] = Time.now if file_name
    logger.info "Compiled erb template #{cache_name}" if logger
  end
  klass
end

def rhtml_render(extension, template, file_name, local_assigns)
  compile_erb_template(template, file_name).new(local_assigns).result
end

We could take the same approach with rxml rendering also.

class CompiledBuilderTemplate < CompiledTemplate
  attr_reader :xml

  def initialize(local_assigns = {})
    @xml = Builder::XmlMarkup.new(:indent => 2)
    super
  end
end

# Same as above for ActionView::Base rxml rendering

What do you think?

Hm. I have two objections: a) I think it does not work b) I think it is less efficient. (But would have to be measured to find out)

It does not work, because variables of the view class are not easily accessible in CompiledTemplate. And setting @v inside a template will have no effect on other templates. Maybe this could be fixed by fiddeling with method_missing, but this would slow down things again.

I propose to simply save the values of locally assigned variables, and restore them after rendering the template. This takes care of recursive templates easily.

def evaluate_assigns(local_assigns = {})
  @assigns.each { |key, value| instance_variable_set "@#{key}", value }
  saved_locals = {}
  local_assigns.each do |key, value|
    varstr = "@#{key}"
    saved_locals[varstr] = instance_variable_get varstr
    instance_variable_set varstr, value
    key = key.to_sym
    self.class.send :attr_accessor, key unless self.respond_to? key
  end
  saved_locals
end

def rhtml_render(extension, template, file_name, local_assigns)
  render_sym = compile_erb_template(template, file_name)
  saved_locals = evaluate_assigns(local_assigns)
  self.send render_sym
  saved_locals.each{|k,v| instance_variable_set k,v }
end

BTW, I changed the name to evaluate_assigns because it also evaluates @assigns.

Anyway, the cleanest solution would be persuade matz that eval "code" should be identical to just writing code in all contexts. Then we could pass local_assigns to the render function and add the following line to the beginning of each render function:

local_assigns.each {|k,v| eval "#{key}=local_assigns['#{key}']" }

But it seems he doesn't what to do it. Which is a shame, because the way it works now violates his claimed design goal for ruby: "stick to the priniciple of least surprise".

I'd like to know what is holding back this patch.

If you fear name clashes between variables passed in local_assigns and other instance variables, you could change

def evaluate_assigns(local_assigns = {})
  @assigns.each { |key, value| instance_variable_set "@#{key}", value }
  saved_locals = {}
  local_assigns.each do |key, value|
    varstr = "@#{key}"
    saved_locals[varstr] = instance_variable_get varstr
    instance_variable_set varstr, value
    key = key.to_sym
    self.class.send :attr_accessor, key unless self.respond_to? key
  end
  saved_locals
end

to something like

def evaluate_assigns(local_assigns = {})
  @assigns.each { |key, value| instance_variable_set "@#{key}", value }
  saved_locals = {}
  local_assigns.each do |key, value|
    varstr = "@_#{key}_"
    saved_locals[varstr] = instance_variable_get varstr
    instance_variable_set varstr, value
    key = key.to_sym
    unless self.respond_to? key
      self.class.classs_eval "def #{key}; @_#{key}_; end"
      self.class.classs_eval "def #{key}=(v); @_#{key}_= v; end"
    end
  end
  saved_locals
end

I have updated the patch so that it applies to trunk without rejects. It passes all template tests.

performance data:

configuration 1: 07-19.p2.old_templates
  requests=1000, options=-p2
configuration 2: 07-19.p2.compiled_templates
  requests=1000, options=-p2
page                               c1 real   c2 real c1 r/s c2 r/s c1/c2
/empty/index                       2.21935   2.21313  450.6  451.8  1.00
/welcome/index                     2.05144   2.06506  487.5  484.2  0.99
/rezept/index                      2.08804   2.08918  478.9  478.7  1.00
/rezept/myknzlpzl                  2.08847   2.08871  478.8  478.8  1.00
/rezept/show/713                   8.61579   7.00667  116.1  142.7  1.23
/rezept/cat/Hauptspeise            9.18456   7.83212  108.9  127.7  1.17
/rezept/cat/Hauptspeise?page=5     9.27459   8.02018  107.8  124.7  1.16
/rezept/letter/G                   9.32339   7.83231  107.3  127.7  1.19

In addition to the changes already described, it is now possible to set the trim parameters for the ERB compiler through a cattr_accessor on ActionView::Base.

same patch with fixes

Fixed bugs introduced by backporting from perf branch.

Changed one unit test (ActionView::render_template takes 4 params now).