Browse files

Memoize ActionView::Base pick_template and find_partial_path for rend…

…ering duration
  • Loading branch information...
1 parent 8a87d8a commit bc5896e708bf8070835bebe61de03b701fa5e6f7 @josh josh committed Jul 22, 2008
View
2 actionpack/lib/action_controller/base.rb
@@ -1261,6 +1261,8 @@ def template_public?(template_name = default_template_name)
def template_exempt_from_layout?(template_name = default_template_name)
template_name = @template.pick_template(template_name).to_s if @template
@@exempt_from_layout.any? { |ext| template_name =~ ext }
+ rescue ActionView::MissingTemplate
+ false
end
def default_template_name(action_name = self.action_name)
View
7 actionpack/lib/action_view/base.rb
@@ -323,15 +323,18 @@ def pick_template(template_path)
if self.class.warn_cache_misses && logger = ActionController::Base.logger
logger.debug "[PERFORMANCE] Rendering a template that was " +
"not found in view path. Templates outside the view path are " +
- "not cached and result in expensive disk operations. Move this " +
- "file into #{view_paths.join(':')} or add the folder to your " +
+ "not cached and result in expensive disk operations. Move this " +
+ "file into #{view_paths.join(':')} or add the folder to your " +
"view path list"
end
template
end
end
+ extend ActiveSupport::Memoizable
+ memoize :pick_template
+
private
# Renders the template present at <tt>template_path</tt>. The hash in <tt>local_assigns</tt>
# is made available as local variables.
View
9 actionpack/lib/action_view/partials.rb
@@ -102,6 +102,8 @@ module ActionView
#
# As you can see, the <tt>:locals</tt> hash is shared between both the partial and its layout.
module Partials
+ extend ActiveSupport::Memoizable
+
private
def render_partial(partial_path, object_assigns = nil, local_assigns = {}) #:nodoc:
local_assigns ||= {}
@@ -129,14 +131,12 @@ def render_partial_collection(partial_path, collection, partial_spacer_template
local_assigns = local_assigns ? local_assigns.clone : {}
spacer = partial_spacer_template ? render(:partial => partial_spacer_template) : ''
- _paths = {}
- _templates = {}
index = 0
collection.map do |object|
_partial_path ||= partial_path || ActionController::RecordIdentifier.partial_path(object, controller.class.controller_path)
- path = _paths[_partial_path] ||= find_partial_path(_partial_path)
- template = _templates[path] ||= pick_template(path)
+ path = find_partial_path(_partial_path)
+ template = pick_template(path)
local_assigns[template.counter_name] = index
result = template.render_partial(self, object, local_assigns, as)
index += 1
@@ -153,5 +153,6 @@ def find_partial_path(partial_path)
"_#{partial_path}"
end
end
+ memoize :find_partial_path
end
end
View
2 actionpack/lib/action_view/template.rb
@@ -75,7 +75,7 @@ def find_full_path(path, load_paths)
load_paths = Array(load_paths) + [nil]
load_paths.each do |load_path|
file = [load_path, path].compact.join('/')
- return load_path, file if File.exist?(file)
+ return load_path, file if File.exist?(file) && File.file?(file)
@lmarlow
lmarlow added a note Jul 22, 2008

Just use File.file?(file) instead of File.exist?(file) && File.file?(file). File.file?(file_name) returns true if the named file exists and is a regular file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
end
raise MissingTemplate.new(load_paths, path)
end
View
23 actionpack/test/template/compiled_templates_test.rb
@@ -4,7 +4,6 @@
uses_mocha 'TestTemplateRecompilation' do
class CompiledTemplatesTest < Test::Unit::TestCase
def setup
- @view = ActionView::Base.new(ActionController::Base.view_paths, {})
@compiled_templates = ActionView::Base::CompiledTemplates
@compiled_templates.instance_methods.each do |m|
@compiled_templates.send(:remove_method, m) if m =~ /^_run_/
@@ -13,29 +12,33 @@ def setup
def test_template_gets_compiled
assert_equal 0, @compiled_templates.instance_methods.size
- assert_equal "Hello world!", @view.render("test/hello_world.erb")
+ assert_equal "Hello world!", render("test/hello_world.erb")
assert_equal 1, @compiled_templates.instance_methods.size
end
def test_template_gets_recompiled_when_using_different_keys_in_local_assigns
assert_equal 0, @compiled_templates.instance_methods.size
- assert_equal "Hello world!", @view.render("test/hello_world.erb")
- assert_equal "Hello world!", @view.render("test/hello_world.erb", {:foo => "bar"})
+ assert_equal "Hello world!", render("test/hello_world.erb")
+ assert_equal "Hello world!", render("test/hello_world.erb", {:foo => "bar"})
assert_equal 2, @compiled_templates.instance_methods.size
end
def test_compiled_template_will_not_be_recompiled_when_rendered_with_identical_local_assigns
assert_equal 0, @compiled_templates.instance_methods.size
- assert_equal "Hello world!", @view.render("test/hello_world.erb")
+ assert_equal "Hello world!", render("test/hello_world.erb")
ActionView::Template.any_instance.expects(:compile!).never
- assert_equal "Hello world!", @view.render("test/hello_world.erb")
+ assert_equal "Hello world!", render("test/hello_world.erb")
end
- def test_compiled_template_will_always_be_recompiled_when_rendered_if_template_is_outside_cache
+ def test_compiled_template_will_be_recompiled_when_rendered_if_template_is_outside_cache
assert_equal 0, @compiled_templates.instance_methods.size
- assert_equal "Hello world!", @view.render("#{FIXTURE_LOAD_PATH}/test/hello_world.erb")
- ActionView::Template.any_instance.expects(:compile!).times(3)
- 3.times { assert_equal "Hello world!", @view.render("#{FIXTURE_LOAD_PATH}/test/hello_world.erb") }
+ assert_equal "Hello world!", render("#{FIXTURE_LOAD_PATH}/test/hello_world.erb")
+ assert_equal 1, @compiled_templates.instance_methods.size
end
+
+ private
+ def render(*args)
+ ActionView::Base.new(ActionController::Base.view_paths, {}).render(*args)
+ end
end
end

0 comments on commit bc5896e

Please sign in to comment.