From 893e9eb99504705419ad6edac14d00e71cef5f12 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Mon, 9 Feb 2009 14:20:30 -0600 Subject: Improve view rendering performance in development mode and reinstate template recompiling in production [#1909 state:resolved] Signed-off-by: Joshua Peek --- actionpack/lib/action_controller/rescue.rb | 2 +- actionpack/lib/action_view/base.rb | 8 ++- actionpack/lib/action_view/partials.rb | 1 - actionpack/lib/action_view/paths.rb | 17 +++---- actionpack/lib/action_view/renderable.rb | 7 +-- actionpack/lib/action_view/template.rb | 80 ++++++++++++++++-------------- 6 files changed, 59 insertions(+), 56 deletions(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_controller/rescue.rb b/actionpack/lib/action_controller/rescue.rb index ec61715b57..40aa7cdd57 100644 --- a/actionpack/lib/action_controller/rescue.rb +++ b/actionpack/lib/action_controller/rescue.rb @@ -38,7 +38,7 @@ module ActionController #:nodoc: 'ActionView::TemplateError' => 'template_error' } - RESCUES_TEMPLATE_PATH = ActionView::Template::EagerPath.new( + RESCUES_TEMPLATE_PATH = ActionView::Template::Path.new( File.join(File.dirname(__FILE__), "templates")) def self.included(base) #:nodoc: diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 70a0ba91a7..3134807a08 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -182,6 +182,10 @@ module ActionView #:nodoc: # that alert()s the caught exception (and then re-raises it). cattr_accessor :debug_rjs + # Specify whether to check whether modified templates are recompiled without a restart + @@cache_template_loading = false + cattr_accessor :cache_template_loading + attr_internal :request delegate :request_forgery_protection_token, :template, :params, :session, :cookies, :response, :headers, @@ -243,8 +247,8 @@ module ActionView #:nodoc: if options[:layout] _render_with_layout(options, local_assigns, &block) elsif options[:file] - tempalte = self.view_paths.find_template(options[:file], template_format) - tempalte.render_template(self, options[:locals]) + template = self.view_paths.find_template(options[:file], template_format) + template.render_template(self, options[:locals]) elsif options[:partial] render_partial(options) elsif options[:inline] diff --git a/actionpack/lib/action_view/partials.rb b/actionpack/lib/action_view/partials.rb index 9e5e0f786e..6fe4dbf375 100644 --- a/actionpack/lib/action_view/partials.rb +++ b/actionpack/lib/action_view/partials.rb @@ -235,6 +235,5 @@ module ActionView self.view_paths.find_template(path, self.template_format) end - memoize :_pick_partial_template end end diff --git a/actionpack/lib/action_view/paths.rb b/actionpack/lib/action_view/paths.rb index b487bd1aa7..e14b21221c 100644 --- a/actionpack/lib/action_view/paths.rb +++ b/actionpack/lib/action_view/paths.rb @@ -2,11 +2,7 @@ module ActionView #:nodoc: class PathSet < Array #:nodoc: def self.type_cast(obj) if obj.is_a?(String) - if !Object.const_defined?(:Rails) || Rails.configuration.cache_classes - Template::EagerPath.new(obj) - else - Template::Path.new(obj) - end + Template::Path.new(obj) else obj end @@ -36,9 +32,8 @@ module ActionView #:nodoc: super(*objs.map { |obj| self.class.type_cast(obj) }) end - def find_template(original_template_path, format = nil) - return original_template_path if original_template_path.respond_to?(:render) - template_path = original_template_path.sub(/^\//, '') + def find_template(template_path, format = nil) + return template_path if template_path.respond_to?(:render) each do |load_path| if format && (template = load_path["#{template_path}.#{I18n.locale}.#{format}"]) @@ -57,7 +52,11 @@ module ActionView #:nodoc: end end - Template.new(original_template_path, self) + if File.exist?(template_path) + return Template.new(template_path, template_path[0] == 47 ? "" : ".") + end + + raise MissingTemplate.new(self, template_path, format) end end end diff --git a/actionpack/lib/action_view/renderable.rb b/actionpack/lib/action_view/renderable.rb index cb774d8248..c127bb25d6 100644 --- a/actionpack/lib/action_view/renderable.rb +++ b/actionpack/lib/action_view/renderable.rb @@ -18,7 +18,6 @@ module ActionView def compiled_source handler.call(self) end - memoize :compiled_source def method_name_without_locals ['_run', extension, method_segment].compact.join('_') @@ -80,6 +79,8 @@ module ActionView begin ActionView::Base::CompiledTemplates.module_eval(source, filename, 0) + rescue Errno::ENOENT => e + raise e # Missing template file, re-raise for Base to rescue rescue Exception => e # errors from template code if logger = defined?(ActionController) && Base.logger logger.debug "ERROR: compiling #{render_symbol} RAISED #{e}" @@ -90,9 +91,5 @@ module ActionView raise ActionView::TemplateError.new(self, {}, e) end end - - def recompile? - false - end end end diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 575ec7ce1c..ee1b9f2886 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -7,6 +7,11 @@ module ActionView #:nodoc: def initialize(path) raise ArgumentError, "path already is a Path class" if path.is_a?(Path) @path = path.freeze + + @paths = {} + templates_in_path do |template| + load_template(template) + end end def to_s @@ -39,12 +44,7 @@ module ActionView #:nodoc: # etc. A format must be supplied to match a formated file. +hello/index+ # will never match +hello/index.html.erb+. def [](path) - templates_in_path do |template| - if template.accessible_paths.include?(path) - return template - end - end - nil + @paths[path] || find_template(path) end private @@ -57,25 +57,30 @@ module ActionView #:nodoc: def create_template(file) Template.new(file.split("#{self}/").last, self) end - end - class EagerPath < Path - def initialize(path) - super - - @paths = {} - templates_in_path do |template| + def load_template(template) template.load! template.accessible_paths.each do |path| @paths[path] = template end end - @paths.freeze - end - def [](path) - @paths[path] - end + def matching_templates(template_path) + Dir.glob("#{@path}/#{template_path}.*").each do |file| + yield create_template(file) unless File.directory?(file) + end + end + + def find_template(path) + return nil if Base.cache_template_loading || ActionController::Base.allow_concurrency + matching_templates(path) do |template| + if template.accessible_paths.include?(path) + load_template(template) + return template + end + end + nil + end end extend TemplateHandlers @@ -97,9 +102,9 @@ module ActionView #:nodoc: attr_accessor :locale, :name, :format, :extension delegate :to_s, :to => :path - def initialize(template_path, load_paths = []) + def initialize(template_path, load_path) template_path = template_path.dup - @load_path, @filename = find_full_path(template_path, load_paths) + @load_path, @filename = load_path, File.join(load_path, template_path) @base_path, @name, @locale, @format, @extension = split(template_path) @base_path.to_s.gsub!(/\/$/, '') # Push to split method @@ -171,7 +176,6 @@ module ActionView #:nodoc: def source File.read(filename) end - memoize :source def method_segment relative_path.to_s.gsub(/([^a-zA-Z0-9_])/) { $1.ord } @@ -185,25 +189,34 @@ module ActionView #:nodoc: if TemplateError === e e.sub_template_of(self) raise e + elsif Errno::ENOENT === e + raise MissingTemplate.new(view.view_paths, filename.sub("#{RAILS_ROOT}/#{load_path}/", "")) else raise TemplateError.new(self, view.assigns, e) end end def stale? - File.mtime(filename) > mtime - end - - def recompile? - !@cached + !frozen? && mtime < mtime(:reload) end def load! - @cached = true - freeze + reloadable? ? memoize_all : freeze end private + def cached? + Base.cache_template_loading || ActionController::Base.allow_concurrency + end + + def reloadable? + !cached? + end + + def recompile? + reloadable? ? stale? : false + end + def valid_extension?(extension) !Template.registered_template_handler(extension).nil? end @@ -212,15 +225,6 @@ module ActionView #:nodoc: I18n.available_locales.include?(locale.to_sym) end - def find_full_path(path, load_paths) - load_paths = Array(load_paths) + [nil] - load_paths.each do |load_path| - file = load_path ? "#{load_path.to_str}/#{path}" : path - return load_path, file if File.file?(file) - end - raise MissingTemplate.new(load_paths, path) - end - # Returns file split into an array # [base_path, name, locale, format, extension] def split(file) @@ -259,5 +263,5 @@ module ActionView #:nodoc: [base_path, name, locale, format, extension] end - end + end end -- cgit v1.2.3