Merge pull request #603 from bhollis/sitemap_speed

Delay recalculating sitemap until absolutely necessary

middleman-core/lib/middleman-core/cli/build.rb

@@ -242,6 +242,7 @@ module Middleman::Cli
 
       # Double-check for compass sprites
       @app.files.find_new_files((Pathname(@app.source_dir) + @app.images_dir).relative_path_from(@app.root_path))
+      @app.sitemap.ensure_resource_list_updated!
 
       # Sort paths to be built by the above order. This is primarily so Compass can
       # find files in the build folder when it needs to generate sprites for the

middleman-core/lib/middleman-core/core_extensions/file_watcher.rb

@@ -11,7 +11,7 @@ module Middleman
         /^\.sass-cache\//,
         /^\.git\//,
         /^\.gitignore$/,
-        /^\.DS_Store$/,
+        /\.DS_Store/,
         /^build\//,
         /^\.rbenv-.*$/,
         /^Gemfile$/,
@@ -116,17 +116,14 @@ module Middleman
             path = Pathname(path)
             return unless path.exist?
 
-            glob = (path + "**/*").to_s
+            glob = (path + "**").to_s
             subset = @known_paths.select { |p| p.fnmatch(glob) }
 
             ::Middleman::Util.all_files_under(path).each do |filepath|
-              if only_new
+              next if only_new && subset.include?(filepath)
-                next if subset.include?(filepath)
-              else
-                subset.delete(filepath)
-              end
 
-              self.did_change(filepath)
+              subset.delete(filepath)
+              did_change(filepath)
             end
 
             subset.each(&method(:did_delete)) unless only_new

middleman-core/lib/middleman-core/sitemap/extensions/on_disk.rb

@@ -14,7 +14,6 @@ module Middleman
         def initialize(sitemap)
           @sitemap = sitemap
           @app     = @sitemap.app
-
           @file_paths_on_disk = Set.new
 
           scoped_self = self
@@ -22,17 +21,18 @@ module Middleman
 
           # Register file change callback
           @app.files.changed do |file|
-            scoped_self.touch_file(file, !scoped_self.waiting_for_ready)
+            scoped_self.touch_file(file)
           end
 
           # Register file delete callback
           @app.files.deleted do |file|
-            scoped_self.remove_file(file, !scoped_self.waiting_for_ready)
+            scoped_self.remove_file(file)
           end
 
           @app.ready do
             scoped_self.waiting_for_ready = false
-            scoped_self.sitemap.rebuild_resource_list!(:on_disk_ready)
+            # Make sure the sitemap is ready for the first request
+            sitemap.ensure_resource_list_updated!
           end
         end
 
@@ -55,7 +55,13 @@ module Middleman
           # in case one of the other manipulators
           # (like asset_hash) cares about the contents of this file,
           # whether or not it belongs in the sitemap (like a partial)
-          @sitemap.rebuild_resource_list!(:touched_file) if rebuild
+          @sitemap.rebuild_resource_list!(:touched_file)
+
+          unless waiting_for_ready || @app.build?
+            # Force sitemap rebuild so the next request is ready to go.
+            # Skip this during build because the builder will control sitemap refresh.
+            @sitemap.ensure_resource_list_updated!
+          end
         end
 
         # Remove a file from the store
@@ -63,7 +69,12 @@ module Middleman
         # @return [void]
         def remove_file(file, rebuild=true)
           if @file_paths_on_disk.delete?(file)
-            @sitemap.rebuild_resource_list!(:removed_file) if rebuild
+            @sitemap.rebuild_resource_list!(:removed_file)
+            unless waiting_for_ready || @app.build?
+              # Force sitemap rebuild so the next request is ready to go.
+              # Skip this during build because the builder will control sitemap refresh.
+              @sitemap.ensure_resource_list_updated!
+            end
           end
         end
 

middleman-core/lib/middleman-core/sitemap/store.rb

@@ -23,41 +23,32 @@ module Middleman
         @app   = app
         @resources = []
         @_cached_metadata = {}
-        @_lookup_cache = { :path => {}, :destination_path => {} }
         @resource_list_manipulators = []
+        @needs_sitemap_rebuild = true
+        reset_lookup_cache!
 
         # Register classes which can manipulate the main site map list
-        register_resource_list_manipulator(:on_disk, Middleman::Sitemap::Extensions::OnDisk.new(self),  false)
+        register_resource_list_manipulator(:on_disk, Middleman::Sitemap::Extensions::OnDisk.new(self))
 
         # Proxies
-        register_resource_list_manipulator(:proxies, @app.proxy_manager, false)
+        register_resource_list_manipulator(:proxies, @app.proxy_manager)
       end
 
-      # Register a klass which can manipulate the main site map list
+      # Register a klass which can manipulate the main site map list. Best to register
+      # these in a before_configuration or after_configuration hook.
+      #
       # @param [Symbol] name Name of the manipulator for debugging
       # @param [Class, Module] inst Abstract namespace which can update the resource list
-      # @param [Boolean] immediately_rebuild Whether the resource list should be immediately recalculated
       # @return [void]
-      def register_resource_list_manipulator(name, inst, immediately_rebuild=true)
+      def register_resource_list_manipulator(name, inst, unused=true)
         @resource_list_manipulators << [name, inst]
-        rebuild_resource_list!(:registered_new) if immediately_rebuild
+        rebuild_resource_list!(:registered_new)
       end
 
       # Rebuild the list of resources from scratch, using registed manipulators
       # @return [void]
       def rebuild_resource_list!(reason=nil)
-        @resources = @resource_list_manipulators.inject([]) do |result, (_, inst)|
+        @needs_sitemap_rebuild = true
-          newres = inst.manipulate_resource_list(result)
-
-          # Reset lookup cache
-          @_lookup_cache = { :path => {}, :destination_path => {} }
-          newres.each do |resource|
-            @_lookup_cache[:path][resource.path] = resource
-            @_lookup_cache[:destination_path][resource.destination_path] = resource
-          end
-
-          newres
-        end
       end
 
       # Find a resource given its original path
@@ -65,7 +56,8 @@ module Middleman
       # @return [Middleman::Sitemap::Resource]
       def find_resource_by_path(request_path)
         request_path = ::Middleman::Util.normalize_path(request_path)
-        @_lookup_cache[:path][request_path]
+        ensure_resource_list_updated!
+        @_lookup_by_path[request_path]
       end
 
       # Find a resource given its destination path
@@ -73,13 +65,15 @@ module Middleman
       # @return [Middleman::Sitemap::Resource]
       def find_resource_by_destination_path(request_path)
         request_path = ::Middleman::Util.normalize_path(request_path)
-        @_lookup_cache[:destination_path][request_path]
+        ensure_resource_list_updated!
+        @_lookup_by_destination_path[request_path]
       end
 
       # Get the array of all resources
       # @param [Boolean] include_ignored Whether to include ignored resources
       # @return [Array<Middleman::Sitemap::Resource>]
       def resources(include_ignored=false)
+        ensure_resource_list_updated!
         if include_ignored
           @resources
         else
@@ -214,6 +208,36 @@ module Middleman
 
         path
       end
+
+      # Actually update the resource list, assuming anything has called
+      # rebuild_resource_list! since the last time it was run. This is
+      # very expensive!
+      def ensure_resource_list_updated!
+        return unless @needs_sitemap_rebuild
+        @needs_sitemap_rebuild = false
+
+        @app.logger.debug "== Rebuilding resource list"
+
+        @resources = @resource_list_manipulators.inject([]) do |result, (_, inst)|
+          newres = inst.manipulate_resource_list(result)
+
+          # Reset lookup cache
+          reset_lookup_cache!
+          newres.each do |resource|
+            @_lookup_by_path[resource.path] = resource
+            @_lookup_by_destination_path[resource.destination_path] = resource
+          end
+
+          newres
+        end
+      end
+
+      private
+
+      def reset_lookup_cache!
+        @_lookup_by_path = {}
+        @_lookup_by_destination_path = {}
+      end
     end
   end
 end

middleman-more/lib/middleman-more/extensions/asset_hash.rb

@@ -14,6 +14,7 @@ module Middleman
               :asset_hash,
               AssetHashManager.new(self, exts, ignore)
             )
+
             use Middleware, :exts => exts, :middleman_app => self, :ignore => ignore
           end
         end

middleman-more/lib/middleman-more/extensions/directory_indexes.rb

@@ -10,7 +10,7 @@ module Middleman
 
         # Once registered
         def registered(app)
-          app.ready do
+          app.after_configuration do
             sitemap.register_resource_list_manipulator(
               :directory_indexes,
               DirectoryIndexManager.new(self)