Skip to content

Commit

Permalink
Make many parts of Rails lazy. In order to facilitate this,
Browse files Browse the repository at this point in the history
add lazy_load_hooks.rb, which allows us to declare code that
should be run at some later time. For instance, this allows
us to defer requiring ActiveRecord::Base at boot time purely
to apply configuration. Instead, we register a hook that should
apply configuration once ActiveRecord::Base is loaded.

With these changes, brings down total boot time of a
new app to 300ms in production and 400ms in dev.

TODO: rename base_hook
  • Loading branch information
wycats committed Mar 7, 2010
1 parent a424f19 commit 39d6f9e
Show file tree
Hide file tree
Showing 34 changed files with 299 additions and 312 deletions.
1 change: 1 addition & 0 deletions actionmailer/lib/action_mailer.rb
Expand Up @@ -34,6 +34,7 @@
require 'active_support/core_ext/module/attr_internal'
require 'active_support/core_ext/module/delegation'
require 'active_support/core_ext/string/inflections'
require 'active_support/lazy_load_hooks'

This comment has been minimized.

Copy link
@pixeltrix

pixeltrix Mar 11, 2010

Contributor

Is this line necessary? None of the other framework components have it and it's actually ActiveSupport::Autoload that's doing the extending.


module ActionMailer
extend ::ActiveSupport::Autoload
Expand Down
2 changes: 2 additions & 0 deletions actionmailer/lib/action_mailer/base.rb
Expand Up @@ -291,6 +291,8 @@ class Base < AbstractController::Base
:parts_order => [ "text/plain", "text/enriched", "text/html" ]
}.freeze

ActionMailer.run_base_hooks(self)

class << self

def mailer_name
Expand Down
10 changes: 6 additions & 4 deletions actionmailer/lib/action_mailer/railtie.rb
Expand Up @@ -6,19 +6,21 @@ class Railtie < Rails::Railtie
railtie_name :action_mailer

initializer "action_mailer.url_for", :before => :load_environment_config do |app|
ActionMailer::Base.send(:include, app.routes.url_helpers)
ActionMailer.base_hook { include app.routes.url_helpers }
end

require "action_mailer/railties/log_subscriber"
log_subscriber ActionMailer::Railties::LogSubscriber.new

initializer "action_mailer.logger" do
ActionMailer::Base.logger ||= Rails.logger
ActionMailer.base_hook { self.logger ||= Rails.logger }
end

initializer "action_mailer.set_configs" do |app|
app.config.action_mailer.each do |k,v|
ActionMailer::Base.send "#{k}=", v
ActionMailer.base_hook do
app.config.action_mailer.each do |k,v|
send "#{k}=", v
end
end
end
end
Expand Down
2 changes: 2 additions & 0 deletions actionpack/lib/action_controller/base.rb
Expand Up @@ -58,6 +58,8 @@ def self.filter_parameter_logging(*args, &block)
filter
end

ActionController.run_base_hooks(self)

end
end

Expand Down
14 changes: 9 additions & 5 deletions actionpack/lib/action_controller/railtie.rb
Expand Up @@ -40,7 +40,7 @@ class Railtie < Rails::Railtie
log_subscriber ActionController::Railties::LogSubscriber.new

initializer "action_controller.logger" do
ActionController::Base.logger ||= Rails.logger
ActionController.base_hook { self.logger ||= Rails.logger }
end

initializer "action_controller.set_configs" do |app|
Expand All @@ -51,19 +51,23 @@ class Railtie < Rails::Railtie
ac.stylesheets_dir = paths.public.stylesheets.to_a.first
ac.secret = app.config.cookie_secret

ActionController::Base.config.replace(ac)
ActionController.base_hook { self.config.replace(ac) }
end

initializer "action_controller.initialize_framework_caches" do
ActionController::Base.cache_store ||= RAILS_CACHE
ActionController.base_hook { self.cache_store ||= RAILS_CACHE }
end

initializer "action_controller.set_helpers_path" do |app|
ActionController::Base.helpers_path = app.config.paths.app.helpers.to_a
ActionController.base_hook do
self.helpers_path = app.config.paths.app.helpers.to_a
end
end

initializer "action_controller.url_helpers" do |app|
ActionController::Base.extend ::ActionController::Railtie::UrlHelpers.with(app.routes)
ActionController.base_hook do
extend ::ActionController::Railtie::UrlHelpers.with(app.routes)
end

message = "ActionController::Routing::Routes is deprecated. " \
"Instead, use Rails.application.routes"
Expand Down
1 change: 0 additions & 1 deletion actionpack/lib/action_dispatch/middleware/params_parser.rb
@@ -1,4 +1,3 @@
require 'active_support/json'
require 'action_dispatch/http/request'

module ActionDispatch
Expand Down
@@ -1,5 +1,4 @@
require 'active_support/core_ext/hash/keys'
require 'rack/request'

module ActionDispatch
module Session
Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_dispatch/middleware/stack.rb
Expand Up @@ -58,7 +58,7 @@ def ==(middleware)
if lazy_compare?(@klass) && lazy_compare?(middleware)
normalize(@klass) == normalize(middleware)
else
klass == ActiveSupport::Inflector.constantize(middleware.to_s)
klass.name == middleware.to_s
end
end
end
Expand Down
4 changes: 3 additions & 1 deletion actionpack/lib/action_dispatch/routing/mapper.rb
@@ -1,3 +1,5 @@
require "active_support/core_ext/hash/except"

module ActionDispatch
module Routing
class Mapper
Expand Down Expand Up @@ -85,7 +87,7 @@ def conditions
end

def requirements
@requirements ||= returning(@options[:constraints] || {}) do |requirements|
@requirements ||= (@options[:constraints] || {}).tap do |requirements|
requirements.reverse_merge!(@scope[:constraints]) if @scope[:constraints]
@options.each { |k, v| requirements[k] = v if v.is_a?(Regexp) }
end
Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_view.rb
Expand Up @@ -41,6 +41,7 @@ module ActionView
autoload :Rendering
end

autoload :Base
autoload :MissingTemplate, 'action_view/base'
autoload :Resolver, 'action_view/template/resolver'
autoload :PathResolver, 'action_view/template/resolver'
Expand All @@ -56,6 +57,5 @@ module ActionView
end

require 'active_support/core_ext/string/output_safety'
require 'action_view/base'

I18n.load_path << "#{File.dirname(__FILE__)}/action_view/locale/en.yml"
2 changes: 2 additions & 0 deletions actionpack/lib/action_view/base.rb
Expand Up @@ -177,6 +177,8 @@ module Subclasses

extend ActiveSupport::Memoizable

ActionView.run_base_hooks(self)

attr_accessor :base_path, :assigns, :template_extension
attr_internal :captures

Expand Down
8 changes: 5 additions & 3 deletions actionpack/lib/action_view/helpers/active_model_helper.rb
Expand Up @@ -5,9 +5,11 @@
require 'active_support/core_ext/kernel/reporting'

module ActionView
class Base
@@field_error_proc = Proc.new{ |html_tag, instance| "<div class=\"fieldWithErrors\">#{html_tag}</div>".html_safe }
cattr_accessor :field_error_proc
ActionView.base_hook do
class ActionView::Base
@@field_error_proc = Proc.new{ |html_tag, instance| "<div class=\"fieldWithErrors\">#{html_tag}</div>".html_safe }
cattr_accessor :field_error_proc
end
end

module Helpers
Expand Down
8 changes: 5 additions & 3 deletions actionpack/lib/action_view/helpers/form_helper.rb
Expand Up @@ -1211,8 +1211,10 @@ def nested_child_index(name)
end
end

class Base
cattr_accessor :default_form_builder
@@default_form_builder = ::ActionView::Helpers::FormBuilder
ActionView.base_hook do
class ActionView::Base
cattr_accessor :default_form_builder
@@default_form_builder = ::ActionView::Helpers::FormBuilder
end
end
end
4 changes: 3 additions & 1 deletion actionpack/lib/action_view/railtie.rb
Expand Up @@ -10,7 +10,9 @@ class Railtie < Rails::Railtie

initializer "action_view.cache_asset_timestamps" do |app|
unless app.config.cache_classes
ActionView::Helpers::AssetTagHelper.cache_asset_timestamps = false
ActionView.base_hook do
ActionView::Helpers::AssetTagHelper.cache_asset_timestamps = false
end
end
end
end
Expand Down
1 change: 1 addition & 0 deletions actionpack/lib/action_view/test_case.rb
@@ -1,4 +1,5 @@
require 'action_controller/test_case'
require 'action_view'

module ActionView
class Base
Expand Down
1 change: 0 additions & 1 deletion actionpack/test/abstract_unit.rb
Expand Up @@ -16,7 +16,6 @@
require 'abstract_controller'
require 'action_controller'
require 'action_view'
require 'action_view/base'
require 'action_dispatch'
require 'fixture_template'
require 'active_support/dependencies'
Expand Down
13 changes: 8 additions & 5 deletions activerecord/lib/active_record.rb
Expand Up @@ -30,16 +30,15 @@

require 'active_support'
require 'active_model'
require 'arel'

module ActiveRecord
extend ActiveSupport::Autoload

eager_autoload do
autoload :VERSION

autoload :ActiveRecordError, 'active_record/base'
autoload :ConnectionNotEstablished, 'active_record/base'
autoload :ActiveRecordError, 'active_record/errors'
autoload :ConnectionNotEstablished, 'active_record/errors'

autoload :Aggregations
autoload :AssociationPreload
Expand Down Expand Up @@ -106,12 +105,16 @@ module ConnectionAdapters

eager_autoload do
autoload :AbstractAdapter
autoload :ConnectionManagement, "active_record/connection_adapters/abstract/connection_pool"
end
end

autoload :TestCase
autoload :TestFixtures, 'active_record/fixtures'

base_hook do
Arel::Table.engine = Arel::Sql::Engine.new(self)
end
end

Arel::Table.engine = Arel::Sql::Engine.new(ActiveRecord::Base)
I18n.load_path << File.dirname(__FILE__) + '/active_record/locale/en.yml'
I18n.load_path << File.dirname(__FILE__) + '/active_record/locale/en.yml'

5 comments on commit 39d6f9e

@michaelklishin
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks very nice already!

@yfeldblum
Copy link

Choose a reason for hiding this comment

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

What effect does this have on threadsafe! mode, if any?

@josh
Copy link
Contributor

@josh josh commented on 39d6f9e Mar 7, 2010

Choose a reason for hiding this comment

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

I'm all for pushing initializer stuff to be as lazy as possible. Nice for console when you may not even use the db connection. Just connect right before we have to.

Using base require as the trigger seems brittle. Anything that loads AR::Base before the initializers run will trigger base hooks with no callbacks. This seems more likely with the suggestion we all should be using Bundler.require.

Almost everything in the framework railties wrap their initializers in this base_hook hook. Why not make it a convention to defer all "active_record.*" initializers till ActiveRecord::Base is loaded? And if its already defined, run them immediately.

And hopefully base_hook is not "public api".

@michaelklishin
Copy link
Contributor

Choose a reason for hiding this comment

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

@yfeldblum: not much if any. This changeset affects (very) early framework loading. The only case for threading at boot time I can think of is initialization of EventMachine reactor in a thread, required by libraries like amqp gem. But EM startup really belongs to initializers, and they are executed later in the process.

@jeremy
Copy link
Member

@jeremy jeremy commented on 39d6f9e Mar 7, 2010

Choose a reason for hiding this comment

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

Josh, could see this being plugin API.

It's natural for a plugin to want to say "if Action Mailer is used, load this stuff" without having to write an initializer that does defined? checks.

Please sign in to comment.