Skip to content
This repository

ActiveSupport::Dependencies#load_missing_constant is slow #1936

Closed
yfeldblum opened this Issue July 01, 2011 · 6 comments

3 participants

Jay Feldblum Aaron Patterson Andrew White
Jay Feldblum

Profiling reveals that ActiveSupport::Dependencies#load_missing_constant is unnecessarily slow in development mode.

In particular, on a mid-sized project, it can consume upwards of 50ms/req by creating a new regexp object complete with interpolation and Regexp.escape thousands of times per request.

The problematic line is: https://github.com/rails/rails/blob/v3.0.9/activesupport/lib/active_support/dependencies.rb#L484.

The recommended solution is: pull the regexp out into a module-level constant. Or, better yet, get a Method object from the regexp's #method(:=~) out into a module-level constant and pass that directly into the block argument of #reject.

Andrew White
Owner

Agreed, this could cause a slowdown in development especially in apps with lots of AR associations as AR::Base.compute_type can end up calling load_missing_constant a few times for each association definition. I would still have some doubts about it being called thousands of times though - it sounds excessive and it does a bunch of file stats that I would've thought overwhelm the savings from this change.

I'd question why we're cleaning the backtrace though - we already have a backtrace cleaner so what's the point of it here? @wycats can you explain?

Jay Feldblum

The block passed to #reject is called (number of lines in the backtrace of #load_missing_constant) * (number of times #load_missing_constant is called). If the backtrace has 60 lines and there are 25 models, that's 1500 calls to the block passed to #reject. Adding middlewares and models only increases this cost. Mongoid loads all models using #load_missing_constant during each request in development.

The regexp in the block passed to #reject is constant in the sense that the regexp in it is the same in every call. That regexp should be a module-level constant rather than be re-created in each call to #load_missing_constant caller.size times. That would slightly refactor the existing implementation, with the tradeoff being the extra memory needed to store one regexp forever but the benefit being that the regexp is created only once during boot and not during any of the requests in development mode rather than being created thousands of times per request in development mode.

Andrew White
Owner

Okay, I can see if you're reloading all of your models then you're going to get a performance hit. If Mongoid reloads everything using #load_missing_constant how does it know which constants to load? Anyway even if you cache the method in a constant you're going to be calling it 1500 times - I still want to know why we're filtering it here.

Jay Feldblum

Mongoid loads all files (using ActiveSupport's mechanism) in app/models/**/*.rb.

Andrew White
Owner

It's using require_dependency so it shouldn't be hitting load_missing_constant - have you got a backtrace showing where this happens?

Jay Feldblum

You're right, most of the model classes are being loaded with require_dependency.

Nevertheless, I am in fact seeing #load_missing_constant being called dozens of times per request. The model classes include modules and validate with validators which ActiveSupport must go out and find with #load_missing_constant, and the controllers generally have multiple ancestors which ActiveSupport must go out and find with #load_missing_constant.

The filter is there for lines 506 and 509 of the same file, which raise the exception with a filtered backtrace, where the filter excludes all lines with /path/to/activesupport/lib/active_support/dependencies.rb to maintain the illusion that activesupport isn't involved and make it easier for users to spot where they made a mistake in referencing a missing constant.

Perhaps that filter should be factored out into a separate method, and lines 506 and 509 changed to call that method and raise the returned exception (with the filtered backtrace). The calls to both #caller and #reject would be moved to that method. This would be even better since #caller would only be called in case the constant actually can't be found. In which case I would certainly like to see the result of #caller and fix the mistake. But in normal operation, where the constant is indeed found, #caller would not be called at all from #load_missing_constant.

Aaron Patterson tenderlove closed this in b9f6798 July 29, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.