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

Long (1+ minute) boot time due to Reloader.safe_load #2178

Open
ramirezd42 opened this issue Feb 22, 2018 · 4 comments
Open

Long (1+ minute) boot time due to Reloader.safe_load #2178

ramirezd42 opened this issue Feb 22, 2018 · 4 comments

Comments

@ramirezd42
Copy link

Do you want to request a feature or report a bug?

Not sure to classify this as bug or feature. Mostly just want to discuss some trouble I'm having with a legacy Padrino app

What is the current behavior?

Our main Padrino app consistently takes around a full minute to boot to a console!
For context we have:

  • 327 model files
  • 34 controllers
  • 322 lib files
  • These files are broken out across 36 different sub apps.

The long boot time is beginning to cause issues so we're investigating ways to speed it up. I've noticed that by commenting out the call to Reloader.safe_load from within require_dependencies and replacing it with a naive require file, I'm able to speed up boot time to around 5 seconds (yes please!) and everything seems to be running more or less as expected.

Quite literally

def require_dependencies(*paths)
      options = paths.extract_options!.merge( :cyclic => true )
      files = paths.flatten.flat_map{ |path| Dir.glob(path).sort_by{ |filename| filename.count('/') } }.uniq

      until files.empty?
        error = fatal = loaded = nil

        files.dup.each do |file|
          begin
            # Reloader.safe_load(file, options)
            require file
            files.delete(file)
            loaded = true
          rescue NameError, LoadError => error
            logger.devel "Cyclic dependency reload for #{error.class}: #{error.message}"
          rescue Exception => fatal
            break
          end
        end

        if fatal || !loaded
          exception = fatal || error
          logger.exception exception, :short
          raise exception
        end
      end
    end

Unfortunately I'm not very familiar with the internals of Padrino. The biggest downside I see to this is that reload! will no longer work, although I could be missing something obvious here.

I'm wondering if
a) I'm missing the bigger picture here and Reloader.safe_load is slow for some other reason
b) I might run into bigger issues down the road by bypassing Reloader.safe_load. If not, maybe there could be a configuration option in place to bypass this for those that would prefer? Happy to submit a PR if that's something that would be welcomed.

What is the expected behavior?

Ideally the app would boot faster. Alternatively I'd like a way of bypassing safe_load in require_dependencies. Or possibly support for some type of caching mechanism like with https://github.com/Shopify/bootsnap

It's much more important to me to have a fast initial boot of our application than it is to have a lightning quick reload time.

Which versions of Ruby, Padrino, Sinatra, Rack, OS are you using? Did this work in previous versions?

Ruby 2.3.1, Padrino: Tried 0.12.8 and 0.14.8

@henrahmagix
Copy link

henrahmagix commented Apr 23, 2018

Similarly, I have found with ruby 2.4.2 and padrino 0.14.1.1 my load speed reduced from 35s to 8s using this exact change, which allows me to iterate far, far quicker on tests!

@cheesysam
Copy link

Hi @ramirezd42, I'm another interested party in speeding this up. Have you had any more discoveries since your first comment?

@basex
Copy link
Contributor

basex commented Apr 30, 2018

Could we only use Reloader.safe_load in development and in the remaining environments use the regular require?

@pchaganti
Copy link

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants