diff options
author | David Heinemeier Hansson <david@loudthinking.com> | 2005-07-21 07:14:35 +0000 |
---|---|---|
committer | David Heinemeier Hansson <david@loudthinking.com> | 2005-07-21 07:14:35 +0000 |
commit | bd558ef98e17e27da51aee77fe50ef2cd129bb6e (patch) | |
tree | a421c2489f33167c4e3157c288c2d42367374a11 | |
parent | d886ad5c8b6f9eaa8b82e634c989aee48ccaf240 (diff) | |
download | rails-bd558ef98e17e27da51aee77fe50ef2cd129bb6e.tar.gz rails-bd558ef98e17e27da51aee77fe50ef2cd129bb6e.tar.bz2 rails-bd558ef98e17e27da51aee77fe50ef2cd129bb6e.zip |
Improved rendering speed on complicated templates by up to 25% #1234 [Stephan Kaes]. This did necessasitate a change to the internals of ActionView#render_template that now has four parameters. Developers of custom view handlers (like Amrita) need to update for that.
git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@1874 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
-rw-r--r-- | actionpack/CHANGELOG | 2 | ||||
-rwxr-xr-x | actionpack/lib/action_controller/base.rb | 1 | ||||
-rw-r--r-- | actionpack/lib/action_view/base.rb | 133 | ||||
-rw-r--r-- | actionpack/test/abstract_unit.rb | 2 | ||||
-rw-r--r-- | actionpack/test/controller/custom_handler_test.rb | 4 | ||||
-rw-r--r-- | actionpack/test/controller/new_render_test.rb | 22 | ||||
-rw-r--r-- | actionpack/test/controller/test_test.rb | 1 | ||||
-rw-r--r-- | actionpack/test/fixtures/test/_person.rhtml | 2 | ||||
-rw-r--r-- | actionpack/test/fixtures/test/potential_conflicts.rhtml | 4 |
9 files changed, 119 insertions, 52 deletions
diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index a09380c835..ac53fb277e 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Improved rendering speed on complicated templates by up to 25% #1234 [Stephan Kaes]. This did necessasitate a change to the internals of ActionView#render_template that now has four parameters. Developers of custom view handlers (like Amrita) need to update for that. + * Added options hash as third argument to FormHelper#input, so you can do input('person', 'zip', :size=>10) #1719 [jeremye@bsa.ca.gov] * Added Base#expires_in(seconds)/Base#expires_now to control HTTP content cache headers #1755 [Thomas Fuchs] diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index ac733c1d36..d19990d155 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -596,6 +596,7 @@ module ActionController #:nodoc: @template.render_template( options[:type] || :rhtml, options[:inline], + nil, options[:locals] || {} ) })) diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index c3c76fb539..4869f8f555 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -130,7 +130,13 @@ module ActionView #:nodoc: @@cache_template_loading = false cattr_accessor :cache_template_loading + # Specify trim mode for the ERB compiler. Defaults to '-'. + # See ERB documentation for suitable values. + @@erb_trim_mode = '-' + cattr_accessor :erb_trim_mode + @@compiled_erb_templates = {} + @@erb_count = 0 @@loaded_templates = {} @@template_handlers = {} @@ -151,6 +157,7 @@ module ActionView #:nodoc: def initialize(base_path = nil, assigns_for_first_render = {}, controller = nil)#:nodoc: @base_path, @assigns = base_path, assigns_for_first_render @controller = controller + @logger = controller && controller.logger end # Renders the template present at <tt>template_path</tt>. If <tt>use_full_path</tt> is set to true, @@ -164,13 +171,13 @@ module ActionView #:nodoc: template_file_name = full_template_path(template_path, template_extension) else template_file_name = template_path - template_extension = template_path.split(".").last + template_extension = template_path.split('.').last end - template_source = read_template_file(template_file_name) + template_source = read_template_file(template_file_name, template_extension) begin - render_template(template_extension, template_source, local_assigns) + render_template(template_extension, template_source, template_file_name, local_assigns) rescue Exception => e if TemplateError === e e.sub_template_of(template_file_name) @@ -204,41 +211,38 @@ module ActionView #:nodoc: # Renders the +template+ which is given as a string as either rhtml or rxml depending on <tt>template_extension</tt>. # The hash in <tt>local_assigns</tt> is made available as local variables. - def render_template(template_extension, template, local_assigns = {}) - send(pick_rendering_method(template_extension), template_extension, - template, local_assigns) + def render_template(template_extension, template, file_name = nil, local_assigns = {}) + if handler = @@template_handlers[template_extension] + delegate_render(handler, template, local_assigns) + elsif template_extension == 'rxml' + rxml_render(template_extension, template, file_name, local_assigns) + else + rhtml_render(template_extension, template, file_name, local_assigns) + end end def pick_template_extension(template_path)#:nodoc: if match = delegate_template_exists?(template_path) match.first elsif erb_template_exists?(template_path) - "rhtml" + 'rhtml' elsif builder_template_exists?(template_path) - "rxml" + 'rxml' else raise ActionViewError, "No rhtml, rxml, or delegate template found for #{template_path}" end end - - def pick_rendering_method(template_extension)#:nodoc: - if @@template_handlers[template_extension] - "delegate_render" - else - (template_extension.to_s == "rxml" ? "rxml" : "rhtml") + "_render" - end - end - + def delegate_template_exists?(template_path)#:nodoc: @@template_handlers.find { |k,| template_exists?(template_path, k) } end def erb_template_exists?(template_path)#:nodoc: - template_exists?(template_path, "rhtml") + template_exists?(template_path, 'rhtml') end def builder_template_exists?(template_path)#:nodoc: - template_exists?(template_path, "rxml") + template_exists?(template_path, 'rxml') end def file_exists?(template_path)#:nodoc: @@ -247,7 +251,7 @@ module ActionView #:nodoc: # Returns true is the file may be rendered implicitly. def file_public?(template_path)#:nodoc: - template_path.split("/").last[0,1] != "_" + template_path.split('/').last[0,1] != '_' end private @@ -256,43 +260,86 @@ module ActionView #:nodoc: end def template_exists?(template_path, extension) - (cache_template_loading && @@loaded_templates.has_key?(template_path)) || - FileTest.exists?(full_template_path(template_path, extension)) + fp = full_template_path(template_path, extension) + (@@cache_template_loading && @@loaded_templates.has_key?(fp)) || FileTest.exists?(fp) end - def read_template_file(template_path) - unless cache_template_loading && @@loaded_templates[template_path] - @@loaded_templates[template_path] = File.read(template_path) + def read_template_file(template_path, extension) + info = @@loaded_templates[template_path] + read_file = info.nil? + read_file ||= info.is_a?(Time) && info<File.stat(template_path).mtime unless @@cache_template_loading + if read_file + @@loaded_templates[template_path] = info = File.read(template_path) + @@compiled_erb_templates[template_path] = nil if 'rhtml' == extension end - - @@loaded_templates[template_path] + info end - def evaluate_locals(local_assigns = {}) - b = binding - - local_assigns.stringify_keys! - local_assigns.each { |key, value| eval "#{key} = local_assigns[\"#{key}\"]", b } + def evaluate_assigns(local_assigns = {}) @assigns.each { |key, value| instance_variable_set "@#{key}", value } - xml = Builder::XmlMarkup.new(:indent => 2) + saved_locals = {} + local_assigns.each do |key, value| + varstr = "@_#{key}_" + saved_locals[varstr] = instance_variable_get varstr + instance_variable_set varstr, value + unless self.respond_to? key + self.class.class_eval "def #{key}; #{varstr}; end" + self.class.class_eval "def #{key}=(v); #{varstr} = v; end" + end + end + #if logger + # logger.info "assigns: #{@assigns.keys.join', '}" + # logger.info "local_assigns: [#{local_assigns.keys.join', '}]" if local_assigns + # logger.info "saved_locals: [#{local_assigns.keys.join', '}]" if saved_locals + #end + saved_locals + end + + def compile_erb_template(template, file_name) + cache_name = file_name || template + unless @@compiled_erb_templates[cache_name] + erb = ERB.new(template, nil, @@erb_trim_mode) + erb_name = 'run_erb_' + if file_name + i = file_name.index(@base_path) + l = @base_path.length + s_file_name = i ? file_name[i+l+1,file_name.length-l-1] : file_name + s_file_name.gsub!(/\/|:/, '_') + s_file_name.sub!(/.rhtml/,'') + erb_name += s_file_name + else + @@erb_count += 1 + erb_name += @@erb_count.to_s + end + erb_def = "def #{erb_name}; #{erb.src}; end" + eval erb_def rescue raise ActionViewError, "ERROR defining #{erb_name}: #{erb_def}" - b + @@compiled_erb_templates[cache_name] = erb_name.intern + @@loaded_templates[cache_name] = Time.now if file_name + logger.info "Compiled erb template #{cache_name}\n ==> #{erb_name}" if logger + end + @@compiled_erb_templates[cache_name] end - def rhtml_render(extension, template, local_assigns) - b = evaluate_locals(local_assigns) - @@compiled_erb_templates[template] ||= ERB.new(template, nil, '-') - @@compiled_erb_templates[template].result(b) + def rhtml_render(extension, template, file_name, local_assigns) + render_sym = compile_erb_template(template, file_name) + saved_locals = evaluate_assigns(local_assigns) + result = self.send render_sym + saved_locals.each{|k,v| instance_variable_set k,v } + result end - def rxml_render(extension, template, local_assigns) + def rxml_render(extension, template, file_name, local_assigns) @controller.headers["Content-Type"] ||= 'text/xml' - eval(template, evaluate_locals(local_assigns), '(template)(eval)', 1) + saved_locals = evaluate_assigns(local_assigns) + xml = Builder::XmlMarkup.new(:indent => 2) + result = eval(template, binding, '(template)(eval)', 1) + saved_locals.each{|k,v| instance_variable_set k,v } + result end - def delegate_render(extension, template, local_assigns) - delegator = @@template_handlers[extension].new(self) - delegator.render(template, local_assigns) + def delegate_render(handler, template, local_assigns) + handler.new(self).render(template, local_assigns) end end end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index fae40cd8ac..5c797a4434 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -8,5 +8,5 @@ require 'action_controller' require 'action_controller/test_process' ActionController::Base.logger = nil -ActionController::Base.ignore_missing_templates = true +ActionController::Base.ignore_missing_templates = false ActionController::Routing::Routes.reload rescue nil diff --git a/actionpack/test/controller/custom_handler_test.rb b/actionpack/test/controller/custom_handler_test.rb index 8939d19166..38e675ffed 100644 --- a/actionpack/test/controller/custom_handler_test.rb +++ b/actionpack/test/controller/custom_handler_test.rb @@ -19,7 +19,7 @@ class CustomHandlerTest < Test::Unit::TestCase end def test_custom_render - result = @view.render_template( "foo", "hello <%= one %>", "one" => "two" ) + result = @view.render_template( "foo", "hello <%= one %>", nil, "one" => "two" ) assert_equal( [ "hello <%= one %>", { "one" => "two" }, @view ], result ) @@ -27,7 +27,7 @@ class CustomHandlerTest < Test::Unit::TestCase def test_unhandled_extension # uses the ERb handler by default if the extension isn't recognized - result = @view.render_template( "bar", "hello <%= one %>", "one" => "two" ) + result = @view.render_template( "bar", "hello <%= one %>", nil, "one" => "two" ) assert_equal "hello two", result end end diff --git a/actionpack/test/controller/new_render_test.rb b/actionpack/test/controller/new_render_test.rb index 67529a825e..105e5ec06d 100644 --- a/actionpack/test/controller/new_render_test.rb +++ b/actionpack/test/controller/new_render_test.rb @@ -114,6 +114,11 @@ class NewRenderTestController < ActionController::Base redirect_to :action => "double_render" end + def rendering_with_conflicting_local_vars + @name = "David" + render :action => "potential_conflicts" + end + def rescue_action(e) raise end private @@ -236,11 +241,6 @@ class NewRenderTest < Test::Unit::TestCase assert_equal "<wrapper>\n<html>\n <p>Hello </p>\n<p>This is grand!</p>\n</html>\n</wrapper>\n", @response.body end - # def test_partials_list - # get :partials_list - # assert_equal "goodbyeHello: davidHello: marygoodbye\n", @response.body - # end - def test_partial_only get :partial_only assert_equal "only partial", @response.body @@ -287,4 +287,14 @@ class NewRenderTest < Test::Unit::TestCase def test_render_and_redirect assert_raises(ActionController::DoubleRenderError) { get :render_and_redirect } end -end + + def test_rendering_with_conflicting_local_vars + get :rendering_with_conflicting_local_vars + assert_equal("First: David\nSecond: Stephan\nThird: David\nFourth: David\nFifth: ", @response.body) + end + + # def test_partials_list + # get :partials_list + # assert_equal "goodbyeHello: davidHello: marygoodbye\n", @response.body + # end +end
\ No newline at end of file diff --git a/actionpack/test/controller/test_test.rb b/actionpack/test/controller/test_test.rb index 0ebd6bc472..b27f3e9423 100644 --- a/actionpack/test/controller/test_test.rb +++ b/actionpack/test/controller/test_test.rb @@ -5,6 +5,7 @@ class TestTest < Test::Unit::TestCase class TestController < ActionController::Base def set_flash flash["test"] = ">#{flash["test"]}<" + render :text => 'ignore me' end def test_params diff --git a/actionpack/test/fixtures/test/_person.rhtml b/actionpack/test/fixtures/test/_person.rhtml new file mode 100644 index 0000000000..b2e5688956 --- /dev/null +++ b/actionpack/test/fixtures/test/_person.rhtml @@ -0,0 +1,2 @@ +Second: <%= name %> +Third: <%= @name %> diff --git a/actionpack/test/fixtures/test/potential_conflicts.rhtml b/actionpack/test/fixtures/test/potential_conflicts.rhtml new file mode 100644 index 0000000000..a5e964e359 --- /dev/null +++ b/actionpack/test/fixtures/test/potential_conflicts.rhtml @@ -0,0 +1,4 @@ +First: <%= @name %> +<%= render :partial => "person", :locals => { :name => "Stephan" } -%> +Fourth: <%= @name %> +Fifth: <%= name %>
\ No newline at end of file |