Skip to content

Commit

Permalink
Optimize globbed file lookups, fixes nasty performance regression
Browse files Browse the repository at this point in the history
  • Loading branch information
tdreyno committed Aug 25, 2014
1 parent 6ef96cc commit 84acb50
Show file tree
Hide file tree
Showing 14 changed files with 52 additions and 60 deletions.
3 changes: 2 additions & 1 deletion middleman-cli/lib/middleman-cli/build.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ def build

builder = Middleman::Builder.new(@app,
glob: options['glob'],
clean: options['clean'])
clean: options['clean'],
parallel: options['parallel'])

builder.on_build_event(&method(:on_event))

Expand Down
18 changes: 0 additions & 18 deletions middleman-core/features/more-instance_vars.feature

This file was deleted.

5 changes: 0 additions & 5 deletions middleman-core/features/partials.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Empty file.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

2 changes: 2 additions & 0 deletions middleman-core/lib/middleman-core/builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ def run!

queue_current_paths if @cleaning
prerender_css

output_files

clean if @cleaning

::Middleman::Profiling.report('build')
Expand Down
3 changes: 1 addition & 2 deletions middleman-core/lib/middleman-core/core_extensions/i18n.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,7 @@ def manipulate_resource_list(resources)

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"
Expand Down
7 changes: 5 additions & 2 deletions middleman-core/lib/middleman-core/sources.rb
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ def by_type(type)
# @return [Array<Middleman::SourceFile>]
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.
Expand All @@ -168,9 +168,10 @@ def files
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

Expand All @@ -182,6 +183,7 @@ def find(type, path, glob=false)
Contract Symbol, String => Bool
def exists?(type, path)
watchers
.lazy
.select { |d| d.type == type }
.any? { |d| d.exists?(path) }
end
Expand All @@ -194,6 +196,7 @@ def exists?(type, path)
Contract Symbol, String => Maybe[HANDLER]
def watcher_for_path(type, path)
watchers
.lazy
.select { |d| d.type == type }
.find { |d| d.exists?(path) }
end
Expand Down
18 changes: 14 additions & 4 deletions middleman-core/lib/middleman-core/sources/source_watcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ def initialize(parent, type, directory, options={})
@directory = Pathname(directory)

@files = {}
@extensionless_files = {}

@validator = options.fetch(:validator, proc { true })
@ignored = options.fetch(:ignored, proc { false })
Expand Down Expand Up @@ -105,8 +106,7 @@ def find(path, glob=false)
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
Expand Down Expand Up @@ -215,7 +215,7 @@ def update(updated_paths, removed_paths)
.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

Expand All @@ -225,13 +225,23 @@ def update(updated_paths, removed_paths)
.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.
Expand Down
1 change: 1 addition & 0 deletions middleman-core/lib/middleman-core/template_context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ def render(_, name, options={}, &block)

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)
Expand Down
44 changes: 27 additions & 17 deletions middleman-core/lib/middleman-core/template_renderer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,24 @@ class TemplateRenderer
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
Expand Down Expand Up @@ -163,21 +176,9 @@ def resolve_template(request_path, options={})
# @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 = []

Expand All @@ -200,7 +201,16 @@ def self.uncached_resolve_template(app, relative_path, options)
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
Expand Down

0 comments on commit 84acb50

Please sign in to comment.