Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Significant Slowdown when Using Debugger #19

Closed
tleish opened this issue Dec 18, 2014 · 8 comments
Closed

Significant Slowdown when Using Debugger #19

tleish opened this issue Dec 18, 2014 · 8 comments

Comments

@tleish
Copy link

tleish commented Dec 18, 2014

When both did_you_mean is enabled and running debugger like debase, I see a significant slowdown in performance than when I just have the debugger running without did_you_mean.

Is it possible to have something like this in development.rb?

DidYouMean.enabled = false if const_defined?(:Debugger)
@yuki24
Copy link
Member

yuki24 commented Dec 18, 2014

Could you elaborate (e.g. what you did and time spent for it with/without did_you_mean, profiling result, version of Ruby, did_you_mean, debase, debugger, other gems, etc)? Also, if you could provide actual performance benchmarks, that would be great.

@tleish
Copy link
Author

tleish commented Dec 18, 2014

Looking into this further, I discovered it's not related to debugging, but rather rails startup. When I run tests with did_you_mean enabled, the Rails startup time takes an additional 67s

Benchmarks:

Rails Startup Time (cache class enabled)

  • 12.471s -- without did_you_mean
  • 79.544s -- with did_you_mean

I've narrowed down the bit of code to gems/ruby-2.1.2/gems/railties-3.2.21/lib/rails/engine.rb#eager_load!. Specifically when the application is loading all the files from Dir.glob("app/controllers/**/*.rb"). In my application, this is 350 controllers.

I'm wondering why enabling/disabling did_you_mean would be causing such a slowdown when eager loading the application controller.

@yuki24
Copy link
Member

yuki24 commented Dec 18, 2014

What version of did_you_mean are you using?

@yuki24
Copy link
Member

yuki24 commented Dec 18, 2014

I'll try to reproduce by creating a fresh rails app and adding 400+ controllers/models.

@tleish
Copy link
Author

tleish commented Dec 18, 2014

I figured out how to reproduce the issue. The problem comes when a Controller does not have a matching Model (e.g. FooController, but no Foo model).

Rails first tries to auto-load the model based on the Controller name:

gems/actionpack-3.2.21/lib/action_controller/metal/params_wrapper.rb#_default_wrap_model

# Determine the wrapper model from the controller's name. By convention,
# this could be done by trying to find the defined model that has the
# same singularize name as the controller. For example, +UsersController+
# will try to find if the +User+ model exists.
#
# This method also does namespace lookup. Foo::Bar::UsersController will
# try to find Foo::Bar::User, Foo::User and finally User.
def _default_wrap_model #:nodoc:
  ...
  model_name = self.name.sub(/Controller$/, '').classify

  begin
    if model_klass = model_name.safe_constantize
      model_klass
    else
      ...
    end
  end until model_klass

  model_klass
end

Then, when running the #safe_constntize on a Model that does not exist, it hits a NameError

gems/activesupport-3.2.21/lib/active_support/inflector/methods.rb#safe_constantize

def safe_constantize(camel_cased_word)
  begin
    constantize(camel_cased_word)
  rescue NameError => e
    raise unless e.message =~ /(uninitialized constant|wrong constant name) #{const_regexp(camel_cased_word)}$/ ||
      e.name.to_s == camel_cased_word.to_s
  rescue ArgumentError => e
    raise unless e.message =~ /not missing constant #{const_regexp(camel_cased_word)}\!$/
  end
end

It's this part of the code that is hitting the did_you_mean gem every time, even though Rails is only testing to see that the Model exists.

With our project we have lots of Controllers that don't necessarily have matching models. did_you_mean is trying to suggest

@tleish
Copy link
Author

tleish commented Dec 18, 2014

FYI, not sure if this is the right solution, but adding the following code fixes the speed issue in my rails application:


class NameError
  ...

  def to_s_with_did_you_mean
    return original_message unless exception.backtrace.grep(/safe_constantize/).empty?
    original_message + did_you_mean?.to_s rescue original_message
  end

  ...
end

It's looking at the backtrace and ignoring anything that's originating from the method safe_constantize, which has the intent to return nil if the constant is not found instead of an error.

See: http://apidock.com/rails/ActiveSupport/Inflector/safe_constantize

@tleish
Copy link
Author

tleish commented Dec 18, 2014

Perhaps you might consider this to be a configuration. The ability to ignore certain methods that might trigger this. Set some defaults like safe_constantize, but allow a developer to add some of their own if they handle dynamic Constants in their methods, similar to safe_constantize. Or, encourage them to use this method of they are using rails.

@yuki24
Copy link
Member

yuki24 commented Dec 21, 2014

fixed by #20. Thanks!

@yuki24 yuki24 closed this as completed Dec 21, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants