Constantize a regexp in Dependencies#load_missing_constant #2279

Merged
merged 1 commit into from Jul 26, 2011

Projects

None yet

3 participants

@jdelStrother
Contributor

Hi,
I stumbled across a potential optimization in Dependencies#load_missing_constant, which currently instantiates a regexp once for every line in Kernel#caller, each time load_missing_constant is called.

We have a relatively large app with a lot of autoloaded constants. After making this regexp constant, we see a noticeable difference to request times when running in development on 1.9.2 - dropping from 4.15s to 3.55s. (The difference in REE was less impressive - 4.96s to 4.88s).

Any comments?
-Jonathan

@spastorino spastorino merged commit 6089ecf into rails:master Jul 26, 2011
@tenderlove tenderlove commented on the diff Jul 26, 2011
activesupport/lib/active_support/dependencies.rb
@@ -478,7 +479,7 @@ module ActiveSupport #:nodoc:
qualified_name = qualified_name_for from_mod, const_name
path_suffix = qualified_name.underscore
- trace = caller.reject {|l| l =~ %r{#{Regexp.escape(__FILE__)}}}
+ trace = caller.reject {|l| l =~ THIS_FILE}
tenderlove
tenderlove Jul 26, 2011 Owner

Why use the regexp at all? It seems like we could replace this with:

trace = caller.reject { |l| l.include? __FILE__ }
jdelStrother
jdelStrother Jul 26, 2011 Contributor

I did wonder about that. Or I suppose

trace = caller.reject { |l| l.starts_with? __FILE__ }

might be a teeny bit faster.

tenderlove
tenderlove Jul 26, 2011 Owner

Do you mind patching, testing, and sending a pull request?

jdelStrother
jdelStrother Jul 26, 2011 Contributor

Sure. I'll try and sort it out now.

tenderlove
tenderlove Jul 26, 2011 Owner

Thanks! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment