Permalink
Browse files

Don't reference ActiveRecord::Base in initializers/wrap_parameters.rb…

…. Use config.active_record instead. This yields about a 20% decrease in startup time because it means that the connection is not created immediately on startup. Of course, this is only useful if you are not going to immediately use the database after startup.
  • Loading branch information...
1 parent 0d5a6f6 commit 4dd985ae9514fdb9688eab780d881decff8358fa @jonleighton jonleighton committed Aug 16, 2011
@@ -3,10 +3,12 @@
# This file contains settings for ActionController::ParamsWrapper which
# is enabled by default.
-# Enable parameter wrapping for JSON. You can disable this by setting :format to an empty array.
-<%= app_const %>.config.wrap_parameters = { <%= key_value :format, "[:json]" %> }
+<%= app_const %>.configure do
+ # Enable parameter wrapping for JSON. You can disable this by setting :format to an empty array.
+ config.action_controller.wrap_parameters = { <%= key_value :format, "[:json]" %> }
-# Disable root element in JSON by default.
-if defined?(ActiveRecord)
- ActiveRecord::Base.include_root_in_json = false
+ <%- unless options.skip_active_record? -%>
+ # Disable root element in JSON by default.
+ config.active_record.include_root_in_json = false
+ <%- end -%>
end

7 comments on commit 4dd985a

Member

josevalim commented on 4dd985a Aug 16, 2011

When the initializers file are loaded, I am almost sure that the configuration options were already copied to ActiveRecord. This means options applied now will have no effect at all. If you want to lazily load ActiveRecord::Base, maybe you should use ActiveSupport.on_load(:active_record) do ... end.

Owner

tenderlove replied Aug 16, 2011

@jonleighton please don't put these kinds of commits on the 3.1 stable branch. We're only fixing bugs. Performance issues (especially boot time) are usually not to be considered bugs.

Member

josevalim replied Aug 16, 2011

In any case, it seems bad to load the connection everytime AR::Base is loaded, no? Maybe we could load the connection just when we are really going to use it?

Owner

tenderlove replied Aug 16, 2011

We don't load the connection until connection is called. I think the issue is slow file loads. @jonleighton c/d?

Member

jonleighton replied Aug 16, 2011

@josevalim, the configs are copied when ActiveSupport.run_load_hooks(:active_record) runs, which happens when ActiveRecord::Base is first initialized. See here. Maybe this could be a problem if the user adds code to reference ActiveRecord::Base before this runs, though.

@tenderlove, I put this in 3-1-stable because there is a bug under the 3.1 milestone complaining about startup performance (#734). If you disagree we can revert but it seemed like an important thing to fix to me.

@josevalim @tenderlove, the connection is established when ActiveRecord::Base is loaded. See here. I agree that fixing this is probably the better way to fix this bug.

Member

jonleighton replied Aug 16, 2011

We should probably invent some sort of lazy config object that will directly assign to ActiveRecord::Base (or whatever) if it is defined, or else will store up the configs and assign them when ActiveRecord::Base becomes defined.

Owner

tenderlove replied Aug 16, 2011

@jonleighton you're right! Sorry about that. :(

Looks like this code is new in 3.1. Are you comfortable with this going out for rc6?

Please sign in to comment.