From 84acb50b0260110f550a42d21f014b780ddc72a2 Mon Sep 17 00:00:00 2001 From: Thomas Reynolds Date: Sun, 24 Aug 2014 17:10:25 -0700 Subject: [PATCH] Optimize globbed file lookups, fixes nasty performance regression --- middleman-cli/lib/middleman-cli/build.rb | 3 +- .../features/more-instance_vars.feature | 18 -------- middleman-core/features/partials.feature | 5 --- .../fixtures/more-instance-vars-app/config.rb | 0 .../source/_vartial.erb | 5 --- .../source/instance-var-set.html.erb | 2 - .../more-instance-vars-app/source/layout.erb | 3 -- .../source/no-instance-var.html.erb | 1 - middleman-core/lib/middleman-core/builder.rb | 2 + .../middleman-core/core_extensions/i18n.rb | 3 +- middleman-core/lib/middleman-core/sources.rb | 7 ++- .../middleman-core/sources/source_watcher.rb | 18 ++++++-- .../lib/middleman-core/template_context.rb | 1 + .../lib/middleman-core/template_renderer.rb | 44 ++++++++++++------- 14 files changed, 52 insertions(+), 60 deletions(-) delete mode 100644 middleman-core/features/more-instance_vars.feature delete mode 100644 middleman-core/fixtures/more-instance-vars-app/config.rb delete mode 100644 middleman-core/fixtures/more-instance-vars-app/source/_vartial.erb delete mode 100644 middleman-core/fixtures/more-instance-vars-app/source/instance-var-set.html.erb delete mode 100644 middleman-core/fixtures/more-instance-vars-app/source/layout.erb delete mode 100644 middleman-core/fixtures/more-instance-vars-app/source/no-instance-var.html.erb diff --git a/middleman-cli/lib/middleman-cli/build.rb b/middleman-cli/lib/middleman-cli/build.rb index 0ec5d234..737d70f8 100644 --- a/middleman-cli/lib/middleman-cli/build.rb +++ b/middleman-cli/lib/middleman-cli/build.rb @@ -60,7 +60,8 @@ module Middleman::Cli builder = Middleman::Builder.new(@app, glob: options['glob'], - clean: options['clean']) + clean: options['clean'], + parallel: options['parallel']) builder.on_build_event(&method(:on_event)) diff --git a/middleman-core/features/more-instance_vars.feature b/middleman-core/features/more-instance_vars.feature deleted file mode 100644 index 87fc3b0d..00000000 --- a/middleman-core/features/more-instance_vars.feature +++ /dev/null @@ -1,18 +0,0 @@ -Feature: Instance Vars - In order to share data with layouts and partials via instance variables - - Scenario: Setting an instance var in a template should be visible in its layout - Given the Server is running at "more-instance-vars-app" - When I go to "/instance-var-set.html" - Then I should see "Var is 100" - - Scenario: Setting an instance var in a template should be visible in a partial - Given the Server is running at "more-instance-vars-app" - When I go to "/instance-var-set.html" - Then I should see "My var is here!" - - Scenario: Setting an instance var in one file should not be visible in another - Given the Server is running at "more-instance-vars-app" - When I go to "/instance-var-set.html" - When I go to "/no-instance-var.html" - Then I should see "No var..." diff --git a/middleman-core/features/partials.feature b/middleman-core/features/partials.feature index 965215a9..86509027 100644 --- a/middleman-core/features/partials.feature +++ b/middleman-core/features/partials.feature @@ -11,11 +11,6 @@ Feature: Provide Sane Defaults for Partial Behavior When I go to "/sub/index.html" Then I should see "Header" And I should see "Footer" - - Scenario: Finds shared partials without _ prefix - Given the Server is running at "partials-app" - When I go to "/using_snippet.html" - Then I should see "Snippet" Scenario: Prefers partials of the same engine type Given the Server is running at "partials-app" diff --git a/middleman-core/fixtures/more-instance-vars-app/config.rb b/middleman-core/fixtures/more-instance-vars-app/config.rb deleted file mode 100644 index e69de29b..00000000 diff --git a/middleman-core/fixtures/more-instance-vars-app/source/_vartial.erb b/middleman-core/fixtures/more-instance-vars-app/source/_vartial.erb deleted file mode 100644 index cdf13355..00000000 --- a/middleman-core/fixtures/more-instance-vars-app/source/_vartial.erb +++ /dev/null @@ -1,5 +0,0 @@ -<% if @my_var %> - My var is here! -<% else %> - No var... -<% end %> diff --git a/middleman-core/fixtures/more-instance-vars-app/source/instance-var-set.html.erb b/middleman-core/fixtures/more-instance-vars-app/source/instance-var-set.html.erb deleted file mode 100644 index d785330f..00000000 --- a/middleman-core/fixtures/more-instance-vars-app/source/instance-var-set.html.erb +++ /dev/null @@ -1,2 +0,0 @@ -<% @my_var = 100 %> -<%= partial 'vartial' %> diff --git a/middleman-core/fixtures/more-instance-vars-app/source/layout.erb b/middleman-core/fixtures/more-instance-vars-app/source/layout.erb deleted file mode 100644 index cbc7ac71..00000000 --- a/middleman-core/fixtures/more-instance-vars-app/source/layout.erb +++ /dev/null @@ -1,3 +0,0 @@ -Var is <%= @my_var %> - -<%= yield %> \ No newline at end of file diff --git a/middleman-core/fixtures/more-instance-vars-app/source/no-instance-var.html.erb b/middleman-core/fixtures/more-instance-vars-app/source/no-instance-var.html.erb deleted file mode 100644 index bcb8a396..00000000 --- a/middleman-core/fixtures/more-instance-vars-app/source/no-instance-var.html.erb +++ /dev/null @@ -1 +0,0 @@ -<%= partial 'vartial' %> diff --git a/middleman-core/lib/middleman-core/builder.rb b/middleman-core/lib/middleman-core/builder.rb index 0e745ea7..c1f693b1 100644 --- a/middleman-core/lib/middleman-core/builder.rb +++ b/middleman-core/lib/middleman-core/builder.rb @@ -50,7 +50,9 @@ module Middleman queue_current_paths if @cleaning prerender_css + output_files + clean if @cleaning ::Middleman::Profiling.report('build') diff --git a/middleman-core/lib/middleman-core/core_extensions/i18n.rb b/middleman-core/lib/middleman-core/core_extensions/i18n.rb index 896ccf3f..0788c9fa 100644 --- a/middleman-core/lib/middleman-core/core_extensions/i18n.rb +++ b/middleman-core/lib/middleman-core/core_extensions/i18n.rb @@ -82,8 +82,7 @@ class Middleman::CoreExtensions::Internationalization < ::Middleman::Extension resources.each do |resource| # If it uses file extension localization - if parse_locale_extension(resource.path) - result = parse_locale_extension(resource.path) + if result = parse_locale_extension(resource.path) ext_lang, path, page_id = result new_resources << build_resource(path, resource.path, page_id, ext_lang) # If it's a "localizable template" diff --git a/middleman-core/lib/middleman-core/sources.rb b/middleman-core/lib/middleman-core/sources.rb index b8e11426..b8050b5f 100644 --- a/middleman-core/lib/middleman-core/sources.rb +++ b/middleman-core/lib/middleman-core/sources.rb @@ -156,7 +156,7 @@ module Middleman # @return [Array] Contract None => ArrayOf[SourceFile] def files - watchers.map(&:files).flatten.uniq { |f| f[:relative_path] } + watchers.flat_map(&:files).uniq { |f| f[:relative_path] } end # Find a file given a type and path. @@ -168,9 +168,10 @@ module Middleman Contract Symbol, String, Maybe[Bool] => Maybe[SourceFile] def find(type, path, glob=false) watchers + .lazy .select { |d| d.type == type } .map { |d| d.find(path, glob) } - .compact + .reject { |d| d.nil? } .first end @@ -182,6 +183,7 @@ module Middleman Contract Symbol, String => Bool def exists?(type, path) watchers + .lazy .select { |d| d.type == type } .any? { |d| d.exists?(path) } end @@ -194,6 +196,7 @@ module Middleman Contract Symbol, String => Maybe[HANDLER] def watcher_for_path(type, path) watchers + .lazy .select { |d| d.type == type } .find { |d| d.exists?(path) } end diff --git a/middleman-core/lib/middleman-core/sources/source_watcher.rb b/middleman-core/lib/middleman-core/sources/source_watcher.rb index e50a2aac..9fb05475 100644 --- a/middleman-core/lib/middleman-core/sources/source_watcher.rb +++ b/middleman-core/lib/middleman-core/sources/source_watcher.rb @@ -43,6 +43,7 @@ module Middleman @directory = Pathname(directory) @files = {} + @extensionless_files = {} @validator = options.fetch(:validator, proc { true }) @ignored = options.fetch(:ignored, proc { false }) @@ -105,8 +106,7 @@ module Middleman p = @directory + p if p.relative? if glob - found = @files.find { |_, v| v[:relative_path].fnmatch(path) } - found ? found.last : nil + @extensionless_files[p] else @files[p] end @@ -215,7 +215,7 @@ module Middleman .select(&method(:valid?)) valid_updates.each do |f| - @files[f[:full_path]] = f + add_file_to_cache(f) logger.debug "== Change (#{f[:type]}): #{f[:relative_path]}" end @@ -225,13 +225,23 @@ module Middleman .select(&method(:valid?)) valid_removes.each do |f| - @files.delete(f[:full_path]) + remove_file_from_cache(f) logger.debug "== Deletion (#{f[:type]}): #{f[:relative_path]}" end run_callbacks(@on_change_callbacks, valid_updates, valid_removes) unless valid_updates.empty? && valid_removes.empty? end + def add_file_to_cache(f) + @files[f[:full_path]] = f + @extensionless_files[f[:full_path].sub_ext('.*')] = f + end + + def remove_file_from_cache(f) + @files.delete(f[:full_path]) + @extensionless_files.delete(f[:full_path].sub_ext('.*')) + end + # Check if this watcher should care about a file. # # @param [Middleman::SourceFile] file The file. diff --git a/middleman-core/lib/middleman-core/template_context.rb b/middleman-core/lib/middleman-core/template_context.rb index 7012e6f9..3021b41e 100644 --- a/middleman-core/lib/middleman-core/template_context.rb +++ b/middleman-core/lib/middleman-core/template_context.rb @@ -102,6 +102,7 @@ module Middleman partial_file = locate_partial(name) + return "" unless partial_file raise ::Middleman::TemplateRenderer::TemplateNotFound, "Could not locate partial: #{name}" unless partial_file source_path = sitemap.file_to_path(partial_file) diff --git a/middleman-core/lib/middleman-core/template_renderer.rb b/middleman-core/lib/middleman-core/template_renderer.rb index dc7e1a77..9da1fae4 100644 --- a/middleman-core/lib/middleman-core/template_renderer.rb +++ b/middleman-core/lib/middleman-core/template_renderer.rb @@ -9,11 +9,24 @@ module Middleman extend Forwardable include Contracts - def self.cache - @_cache ||= ::Tilt::Cache.new + class Cache + def initialize + @cache = {} + end + + def fetch(*key) + @cache[key] = yield unless @cache.key?(key) + @cache[key] + end + + def clear + @cache = {} + end end - def_delegator :"self.class", :cache + def self.cache + @_cache ||= Cache.new + end # Custom error class for handling class TemplateNotFound < RuntimeError; end @@ -163,21 +176,9 @@ module Middleman # @return [String, Boolean] Either the path to the template, or false Contract IsA['Middleman::Application'], Or[Symbol, String], Maybe[Hash] => Maybe[IsA['Middleman::SourceFile']] def self.resolve_template(app, request_path, options={}) - # Find the path by searching or using the cache + # Find the path by searching relative_path = Util.strip_leading_slash(request_path.to_s) - # Cache lookups in build mode only - if app.build? - cache.fetch(:resolve_template, relative_path, options) do - uncached_resolve_template(app, relative_path, options) - end - else - uncached_resolve_template(app, relative_path, options) - end - end - - Contract IsA['Middleman::Application'], String, Hash => Maybe[IsA['Middleman::SourceFile']] - def self.uncached_resolve_template(app, relative_path, options) # By default, any engine will do preferred_engines = [] @@ -200,7 +201,16 @@ module Middleman path_with_ext = relative_path.dup path_with_ext << ('.' + preferred_engine) unless preferred_engine.nil? - file = app.files.find(:source, path_with_ext, preferred_engine == '*') + globbing = preferred_engine == '*' + + # Cache lookups in build mode only + file = if app.build? + cache.fetch(path_with_ext, preferred_engine) do + app.files.find(:source, path_with_ext, globbing) + end + else + app.files.find(:source, path_with_ext, globbing) + end found_template = file if file && (preferred_engine.nil? || ::Tilt[file[:full_path]]) break if found_template