Make reloading of routes optional#4053
Conversation
As has been seen in a previous pull request, some applications require routes to be loaded before the code is eagerly loaded, which implies that all Rails applications using Devise need to have routes reloaded twice: heartcombo#3241 This can incur a very significant slowdown for large apps that have a lot of routes or a lot of controllers, so reloading should be optional.
|
Looks good. Any consideration? cc @lucasmazza @josevalim |
|
|
||
| # When false, Devise will not attempt to reload routes on eager load | ||
| mattr_accessor :reload_routes | ||
| @@reload_routes = true |
There was a problem hiding this comment.
In our template configuration file, we should explicit set it? cc @lucasmazza
There was a problem hiding this comment.
We don't need to set it on the generated config file, but we can add a commented example explaining the purpose of the setting.
|
@lucasmazza @ulissesalmeida what do you think about making this the default value in the release after the next? There are probably much more applications that don't need routes to be reloaded than those which do. |
|
@sidonath I guess we can go down this road but I'm not sure if we can push this into a |
|
Makes sense 😄 Let me know if I should add something here |
|
Hey @sidonath . You can add configuration line and a explanation about this config on the https://github.com/plataformatec/devise/blob/master/lib/generators/templates/devise.rb template file. Explaining what happens when you set |
|
Sure, will do |
|
@lucasmazza thanks for wrapping it up, and sorry about dropping the ball |
3 participants
In #3153 we've seen that some applications require routes to be loaded before the code is eagerly loaded so we can't just remove it. However, this implies that all applications using Devise need to have routes reloaded twice (as was mentioned in #3241 (comment)). Double route reload can incur a slowdown for larger apps that have a lot of routes or a lot of controllers.
This PR strives for compromise, making the route reloading optional.
In our app, enabling this setting shaves off 15-25% of boot time in staging environment (we have more than 1500 routes).