Skip to content

Commit

Permalink
Speed up development by only reloading classes if dependencies files …
Browse files Browse the repository at this point in the history
…changed.

This can be turned off by setting `config.reload_classes_only_on_change` to false.

Extensions like Active Record should add their respective files like db/schema.rb and db/structure.sql to `config.watchable_files` if they want their changes to affect classes reloading.

Thanks to https://github.com/paneq/active_reload and Pastorino for the inspiration. <3
  • Loading branch information
josevalim committed Dec 12, 2011
1 parent 62cda03 commit fa1d9a8
Show file tree
Hide file tree
Showing 15 changed files with 157 additions and 58 deletions.
9 changes: 9 additions & 0 deletions actionpack/test/dispatch/reloader_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,15 @@ def test_manual_reloading
assert cleaned
end

def test_prepend_prepare_callback
i = 10
Reloader.to_prepare { i += 1 }
Reloader.to_prepare(:prepend => true) { i = 0 }

Reloader.prepare!
assert_equal 1, i
end

def test_cleanup_callbacks_are_called_on_exceptions
cleaned = false
Reloader.to_cleanup { cleaned = true }
Expand Down
5 changes: 5 additions & 0 deletions activerecord/lib/active_record/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ class Railtie < Rails::Railtie
end
end

initializer "active_record.add_watchable_files" do |app|
files = ["#{app.root}/db/schema.rb", "#{app.root}/db/structure.sql"]
config.watchable_files.concat files.select { |f| File.exist?(f) }
end

config.after_initialize do
ActiveSupport.on_load(:active_record) do
instantiate_observers
Expand Down
43 changes: 33 additions & 10 deletions activesupport/lib/active_support/file_update_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,30 +39,53 @@ def initialize(paths, calculate=false, &block)
@paths = paths
@glob = compile_glob(@paths.extract_options!)
@block = block
@updated_at = nil
@last_update_at = calculate ? updated_at : nil
end

def updated_at
all = []
all.concat @paths
all.concat Dir[@glob] if @glob
all.map { |path| File.mtime(path) }.max
# Check if any of the entries were updated. If so, the updated_at
# value is cached until flush! is called.
def updated?
current_updated_at = updated_at
if @last_update_at != current_updated_at
@updated_at = updated_at
true
else
false
end
end

# Flush the cache so updated? is calculated again
def flush!
@updated_at = nil
end

# Execute the block given if updated. This call
# always flush the cache.
def execute_if_updated
current_update_at = self.updated_at
if @last_update_at != current_update_at
@last_update_at = current_update_at
if updated?
@last_update_at = updated_at
@block.call
true
else
false
end
ensure
flush!
end

private

def compile_glob(hash)
def updated_at #:nodoc:
@updated_at || begin
all = []
all.concat @paths
all.concat Dir[@glob] if @glob
all.map { |path| File.mtime(path) }.max
end
end

def compile_glob(hash) #:nodoc:
return if hash.empty?
globs = []
hash.each do |key, value|
Expand All @@ -71,7 +94,7 @@ def compile_glob(hash)
"{#{globs.join(",")}}"
end

def compile_ext(array)
def compile_ext(array) #:nodoc:
array = Array.wrap(array)
return if array.empty?
".{#{array.join(",")}}"
Expand Down
3 changes: 2 additions & 1 deletion activesupport/lib/active_support/i18n_railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ def self.reloader
# point, no path was added to the reloader, I18n.reload! is not triggered
# on to_prepare callbacks. This will only happen on the config.after_initialize
# callback below.
initializer "i18n.callbacks" do
initializer "i18n.callbacks" do |app|
app.reloaders << I18n::Railtie.reloader
ActionDispatch::Reloader.to_prepare do
I18n::Railtie.reloader.execute_if_updated
end
Expand Down
13 changes: 13 additions & 0 deletions activesupport/test/file_update_checker_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,19 @@ def test_should_invoke_the_block_if_a_file_has_changed
assert_equal 1, i
end

def test_should_cache_updated_result_until_flushed
i = 0
checker = ActiveSupport::FileUpdateChecker.new(FILES, true){ i += 1 }
assert !checker.updated?

sleep(1)
FileUtils.touch(FILES)

assert checker.updated?
assert checker.execute_if_updated
assert !checker.updated?
end

def test_should_invoke_the_block_if_a_watched_dir_changed_its_glob
i = 0
checker = ActiveSupport::FileUpdateChecker.new([{"tmp_watcher" => [:txt]}], true){ i += 1 }
Expand Down
7 changes: 3 additions & 4 deletions railties/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
## Rails 3.2.0 (unreleased) ##

* New applications get a flag
`config.active_record.auto_explain_threshold_in_seconds` in the environments
configuration files. With a value of 0.5 in development.rb, and commented
out in production.rb. No mention in test.rb. *fxn*
* Speed up development by only reloading classes if dependencies files changed. This can be turned off by setting `config.reload_classes_only_on_change` to false. *José Valim*

* New applications get a flag `config.active_record.auto_explain_threshold_in_seconds` in the environments configuration files. With a value of 0.5 in development.rb, and commented out in production.rb. No mention in test.rb. *fxn*

* Add DebugExceptions middleware which contains features extracted from ShowExceptions middleware *José Valim*

Expand Down
2 changes: 2 additions & 0 deletions railties/guides/source/configuring.textile
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ NOTE. The +config.asset_path+ configuration is ignored if the asset pipeline is

* +config.preload_frameworks+ enables or disables preloading all frameworks at startup. Enabled by +config.threadsafe!+. Defaults to +nil+, so is disabled.

* +config.reload_classes_only_on_change+ enables or disables reloading of classes only when tracked files change. By default tracks everything on autoload paths and is set to true.

* +config.reload_plugins+ enables or disables plugin reloading. Defaults to false.

* +config.secret_token+ used for specifying a key which allows sessions for the application to be verified against a known secure key to prevent tampering. Applications get +config.secret_token+ initialized to a random key in +config/initializers/secret_token.rb+.
Expand Down
40 changes: 38 additions & 2 deletions railties/lib/rails/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,14 @@ def inherited(base)

attr_accessor :assets, :sandbox
alias_method :sandbox?, :sandbox
attr_reader :reloaders

delegate :default_url_options, :default_url_options=, :to => :routes

def initialize
super
@initialized = false
@reloaders = []
end

# This method is called just after an application inherits from Rails::Application,
Expand Down Expand Up @@ -119,17 +121,43 @@ def set_routes_reloader_hook
reloader = routes_reloader
hook = lambda { reloader.execute_if_updated }
hook.call
self.reloaders << reloader
ActionDispatch::Reloader.to_prepare(&hook)
end

# An app dependencies hook that is used to setup to_cleanup callbacks.
# A plugin may override this if they desire to provide a more exquisite app reloading.
# :api: plugin
def set_dependencies_hook
ActionDispatch::Reloader.to_cleanup do
callback = lambda do
ActiveSupport::DescendantsTracker.clear
ActiveSupport::Dependencies.clear
end

if config.reload_classes_only_on_change
reloader = ActiveSupport::FileUpdateChecker.new(watchable_args, true, &callback)
self.reloaders << reloader
# We need to set a to_prepare callback regardless of the reloader result, i.e.
# models should be reloaded if any of the reloaders (i18n, routes) were updated.
ActionDispatch::Reloader.to_prepare(:prepend => true, &callback)
else
ActionDispatch::Reloader.to_cleanup(&callback)
end
end

# Returns an array of file paths appended with a hash of directories-extensions
# suitable for ActiveSupport::FileUpdateChecker API.
def watchable_args
files = []
files.concat config.watchable_files

dirs = {}
dirs.merge! config.watchable_dirs

This comment has been minimized.

Copy link
@spastorino

spastorino Dec 12, 2011

Contributor

dirs = config.watchable_dirs.dup ?

This comment has been minimized.

Copy link
@josevalim

josevalim Dec 12, 2011

Author Contributor

feel free to change it bro :) the line above as well.

ActiveSupport::Dependencies.autoload_paths.each do |path|
dirs[path.to_s] = [:rb]
end

files << dirs
end

# Initialize the application passing the given group. By default, the
Expand Down Expand Up @@ -223,6 +251,10 @@ def helpers_paths #:nodoc:

alias :build_middleware_stack :app

def reload_dependencies?
config.reload_classes_only_on_change != true || reloaders.map(&:updated?).any?
end

def default_middleware_stack
ActionDispatch::MiddlewareStack.new.tap do |middleware|
if rack_cache = config.action_controller.perform_caching && config.action_dispatch.rack_cache
Expand Down Expand Up @@ -252,7 +284,11 @@ def default_middleware_stack
middleware.use ::Rack::Sendfile, config.action_dispatch.x_sendfile_header
end

middleware.use ::ActionDispatch::Reloader unless config.cache_classes
unless config.cache_classes
app = self
middleware.use ::ActionDispatch::Reloader, lambda { app.reload_dependencies? }
end

middleware.use ::ActionDispatch::Callbacks
middleware.use ::ActionDispatch::Cookies

Expand Down
49 changes: 25 additions & 24 deletions railties/lib/rails/application/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,37 +7,38 @@ class Application
class Configuration < ::Rails::Engine::Configuration
attr_accessor :allow_concurrency, :asset_host, :asset_path, :assets,
:cache_classes, :cache_store, :consider_all_requests_local,
:dependency_loading, :filter_parameters,
:force_ssl, :helpers_paths, :logger, :log_tags, :preload_frameworks,
:relative_url_root, :reload_plugins, :secret_token, :serve_static_assets,
:ssl_options, :static_cache_control, :session_options,
:time_zone, :whiny_nils, :railties_order, :all_initializers
:dependency_loading, :filter_parameters, :force_ssl, :helpers_paths,
:initializers_paths, :logger, :log_tags, :preload_frameworks,
:railties_order, :relative_url_root, :reload_plugins, :secret_token,
:serve_static_assets, :ssl_options, :static_cache_control, :session_options,
:time_zone, :reload_classes_only_on_change, :whiny_nils

attr_writer :log_level
attr_reader :encoding

def initialize(*)
super
self.encoding = "utf-8"
@allow_concurrency = false
@consider_all_requests_local = false
@filter_parameters = []
@helpers_paths = []
@dependency_loading = true
@serve_static_assets = true
@static_cache_control = nil
@force_ssl = false
@ssl_options = {}
@session_store = :cookie_store
@session_options = {}
@time_zone = "UTC"
@log_level = nil
@middleware = app_middleware
@generators = app_generators
@cache_store = [ :file_store, "#{root}/tmp/cache/" ]
@railties_order = [:all]
@all_initializers = []
@relative_url_root = ENV["RAILS_RELATIVE_URL_ROOT"]
@allow_concurrency = false
@consider_all_requests_local = false
@filter_parameters = []
@helpers_paths = []
@dependency_loading = true
@serve_static_assets = true
@static_cache_control = nil
@force_ssl = false
@ssl_options = {}
@session_store = :cookie_store
@session_options = {}
@time_zone = "UTC"
@log_level = nil
@middleware = app_middleware
@generators = app_generators
@cache_store = [ :file_store, "#{root}/tmp/cache/" ]
@railties_order = [:all]
@initializers_paths = []
@relative_url_root = ENV["RAILS_RELATIVE_URL_ROOT"]
@reload_classes_only_on_change = true

@assets = ActiveSupport::OrderedOptions.new
@assets.enabled = false
Expand Down
14 changes: 7 additions & 7 deletions railties/lib/rails/application/finisher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module Finisher
$rails_rake_task = nil

initializer :load_config_initializers do
config.all_initializers.each { |init| load(init) }
config.initializers_paths.each { |init| load(init) }
end

initializer :add_generator_templates do
Expand Down Expand Up @@ -64,18 +64,18 @@ module Finisher
ActiveSupport.run_load_hooks(:after_initialize, self)
end

# Set app reload just after the finisher hook to ensure
# paths added in the hook are still loaded.
initializer :set_dependencies_hook, :group => :all do |app|
app.set_dependencies_hook
end

# Set app reload just after the finisher hook to ensure
# routes added in the hook are still loaded.
initializer :set_routes_reloader_hook do |app|
app.set_routes_reloader_hook
end

# Set app reload just after the finisher hook to ensure
# paths added in the hook are still loaded.
initializer :set_dependencies_hook, :group => :all do |app|
app.set_dependencies_hook
end

# Disable dependency loading during request cycle
initializer :disable_dependency_loading do
if config.cache_classes && !config.dependency_loading
Expand Down
12 changes: 4 additions & 8 deletions railties/lib/rails/application/routes_reloader.rb
Original file line number Diff line number Diff line change
@@ -1,21 +1,17 @@
require "active_support/core_ext/module/delegation"

module Rails
class Application
class RoutesReloader
attr_reader :route_sets

delegate :paths, :execute_if_updated, :updated?, :to => :@updater

def initialize(updater=ActiveSupport::FileUpdateChecker)
@updater = updater.new([]) { reload! }
@route_sets = []
end

def paths
@updater.paths
end

def execute_if_updated
@updater.execute_if_updated
end

def reload!
clear!
load_paths
Expand Down
2 changes: 1 addition & 1 deletion railties/lib/rails/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ def load_seed
end

initializer :append_config_initializers do |app|
app.config.all_initializers.concat config.paths["config/initializers"].existent.sort
app.config.initializers_paths.concat config.paths["config/initializers"].existent.sort
end

initializer :engines_blank_point do
Expand Down
12 changes: 12 additions & 0 deletions railties/lib/rails/railtie/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,18 @@ def initialize
@@options ||= {}
end

# Add files that should be watched for change.
def watchable_files
@@watchable_files ||= []
end

# Add directories that should be watched for change.
# The key of the hashes should be directories and the values should
# be an array of extensions to match in each directory.
def watchable_dirs
@@watchable_dirs ||= {}
end

# This allows you to modify the application's middlewares from Engines.
#
# All operations you run on the app_middleware will be replayed on the
Expand Down
3 changes: 2 additions & 1 deletion railties/test/application/console_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ class User

load_environment
assert User.new.respond_to?(:name)
assert !User.new.respond_to?(:age)

sleep(1)

This comment has been minimized.

Copy link
@sobrinho

sobrinho Dec 13, 2011

Contributor

Sleep Driven Development :P

This comment has been minimized.

Copy link
@josevalim

josevalim Dec 13, 2011

Author Contributor

FWIW, the sleep is required because File.mtime just changes after 1s. So if we don't sleep, the reloader cannot pick up file system changes.

This comment has been minimized.

Copy link
@sobrinho

sobrinho Dec 13, 2011

Contributor

Thanks for explanation :)


app_file "app/models/user.rb", <<-MODEL
class User
Expand Down
Loading

12 comments on commit fa1d9a8

@fxn
Copy link
Member

@fxn fxn commented on fa1d9a8 Dec 12, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️ ❤️ ❤️ ❤️ ❤️ ❤️ ❤️

@guilleiguaran
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOVE THIS ❤️

@radar
Copy link
Contributor

@radar radar commented on fa1d9a8 Dec 12, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OMG BLOAT.

trollface.jpg

[<3<3<3<3<3]

@andrewhr
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no sufficient ❤️ to express myself.

@mcmire
Copy link
Contributor

@mcmire mcmire commented on fa1d9a8 Dec 12, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesomesauce. Makes sense to me.

@gavinlaking
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm liking it! So when will see this bad boy in action?

@dmitriy-kiriyenko
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow.

@xiplias
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome

@pokonski
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

! :)

@thilo
Copy link
Contributor

@thilo thilo commented on fa1d9a8 Dec 13, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would save so much time, thank you!

@igorbozato
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WOLOLO

@mattkanwisher
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally !!!!! THX!!!!

Please sign in to comment.