janko / rodauth-rails

Rails integration for Rodauth authentication framework
https://github.com/jeremyevans/rodauth
MIT License
584 stars 40 forks source link

Slow `ActionView::PathResolver#find_template_paths` #48

Closed bjeanes closed 3 years ago

bjeanes commented 3 years ago

I'm not sure if there is a clear fix that will work for all cases, but I wanted to bring this to your attention.

We've have really slow view loading in certain contexts. The glob that it searches for views ends up compiling to:

app/views/rodauth/login{.en,}{.html,.text,.js,.css,.ics,.csv,.vcf,.vtt,.png,.jpeg,.gif,.bmp,.tiff,.svg,.mpeg,.mp3,.ogg,.m4a,.webm,.mp4,.otf,.ttf,.woff,.woff2,.xml,.rss,.atom,.yaml,.multipart_form,.url_encoded_form,.json,.pdf,.zip,.gzip,}{}{.raw,.erb,.html,.builder,.ruby,.coffee,.slim,.jbuilder,.haml,}

I'm supposing we don't have this problem elsewhere because Rails might eliminate options here from information from routing layer, but that's just speculation.

I think somehow incorporating respond_to { html { } } around the renders may cut this down significantly.

For some members of our team (particularly macOS users where Docker's filesystem access is pretty indirect), these renders can be > 300ms.

We are on Rails 5, so perhaps Rails 6 wouldn't have this problem due to https://github.com/rails/rails/pull/33860

We've worked around this by inserting the following into our rodauth controller:

  def _render_template(options)
    variant = options.delete(:variant)
    assigns = options.delete(:assigns)
    context = view_context

    context.assign assigns if assigns
    lookup_context.rendered_format = nil if options[:formats]
    lookup_context.variants = variant if variant

    lookup_context.formats = [:html]
    lookup_context.handlers = [:slim, :erb]

    view_renderer.render(context, options)
  end
janko commented 3 years ago

Thanks for the report. Yes, I would definitely like to add a performance patch for this for Rails 5.x. I'll try to get more familiar with the related Action View code, and see if it's possible to add a safe patch. Using respond_to might be possible, thanks for the tip.

janko commented 3 years ago

Does this problem occur when rendering any template in Rails 5 (in other controllers)? Or just with manually created controller instances used in rodauth-rails?

bjeanes commented 3 years ago

Good question. It was my colleague @chendo who did the benchmarking, so hopefully he can weigh in on that, but it was my understanding from his internal PR that it was just Rodauth. He only added that _render_template call in our RodauthController

chendo commented 3 years ago

It was specifically just Rodauth. Regular Rails controllers in the same project use globs that look like this: "/app/app/views/controller/action{.en,}{.html,}{}{.raw,.erb,.html,.builder,.ruby,.coffee,.slim,.jbuilder,.haml,}"

That particular controller action has no explicit render call, so something in Rails would be limiting the extensions to just .html

janko commented 3 years ago

After some looking, I found out that Action Controller normally limits the template formats for lookup to the ones accepted by the current request (code). This is why classic rendering inside a controller is fast even without using respond_to.

In rodauth-rails case, template rendering is happening outside of action processing, so this code isn't being executed. I've opened https://github.com/janko/rodauth-rails/pull/52 which should fix this by adding this logic. @chendo Could you verify whether it works for you?