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

What is the expected slowdown of using SimpleForm? #1227

Closed
maxwell opened this issue Mar 21, 2015 · 40 comments
Closed

What is the expected slowdown of using SimpleForm? #1227

maxwell opened this issue Mar 21, 2015 · 40 comments

Comments

@maxwell
Copy link

maxwell commented Mar 21, 2015

I was investigating some slow view rendering in our application.

We had a partial that was rendered many times, which looked like this. I had perhaps stupidly opted out of most Simple Form goodness, but I guess for consistency's sake, thought this would be ok.

<div class='sortableItem variant' data-item-id='<%= f.object.id %>'>
  <div class="variant-creator-row">
    <div class="variant-details">
      <%= f.input :text, label: false, wrapper: false, as: :string %>
    </div>
    <div class="variant-option-value">
      <%= f.input :option_1_value, label: false, wrapper: false %>
    </div>
    <div class="variant-option-value">
      <%= f.input :option_2_value, label: false, wrapper: false %>
    </div>
    <div class="variant-option-value">
      <%= f.input :option_3_value, label: false, wrapper: false %>
    </div>

    <div class="variant-hidden">
      <%= f.input :hidden, label: false,  wrapper: false %>
    </div>

    <div class="variant-delete survey-answer-delete">
      <a href="#" class="text text-danger">delete</a>
      <div class="hidden">
        <%= f.input_field :_destroy, as: :boolean, label: false, wrapper: false %>
      </div>
    </div>
  </div>
</div>

<%= f.hidden_field :field_kind, value: 'multiple_choice' %>

Worth noting, this f value is coming from a "simple_fields_for" call.

This partial was getting rendered many times, and looking at my logs, each one was taking > 200 milliseconds! eek!
https://gist.github.com/maxwell/12cd7e949cae4ea60766

Changing this to just be normal Rails helpers radically reduced render time, to about 5 milliseconds per partial, 40x faster!

https://gist.github.com/maxwell/f7b6c1bc6625f6b5c3e7

I realize that it is reasonable to know that there would be some overhead, and perhaps one should not use SimpleForms for simple forms which are rendered many times, but is this the kind of overhead we should expect? Am I doing something incredibly stupid to cause this behavior?

I am currently using simple_form (3.1.0.rc1), and Rails 4.1.10

I love using SimpleForm, thanks so much for all the hard work you guys put into it!

@nashby
Copy link
Collaborator

nashby commented Mar 21, 2015

Hey @maxwell. According to the profiler SimpleForm spends a lot of time for trying to translate hints and placeholders for inputs. I tried to disable them with placeholder: false, hint: false in the f.input call and I got pretty the same results as with regular form_for. I'll try to investigate this further but for now you can disable this components to improve your view perf.

@varyform
Copy link

varyform commented Apr 7, 2015

Same issue here. Forms with 2-3 inputs take up to 400-500ms to render.

@josevalim
Copy link
Contributor

Right. The issue is that, as we add more fallbacks, I18n is just going to be come slower and slower. That's why we had discussions in the past to make this stuff opt-in or reduce the amount of fallbacks.

@rafaelfranca
Copy link
Collaborator

Yeah, we have to make them opt-in

@sevenseacat
Copy link

oh man, just was looking into this because I have a form with over 30 fields and it takes like 20 seconds just to render it. Hoping a fix comes soon :/

@josevalim
Copy link
Contributor

Something is definitely fishy though. Even If it takes about 100ms per field, there is something drastically wrong with I18n. Are you using the default storage or some special backend? Is it reproducible in new applications? We should also remove the amount of fallbacks in our inputs.

@carlosantoniodasilva
Copy link
Member

Can you guys tell me which I18n version you're using? Maybe there's something going on there that we didn't catch.

@varyform
Copy link

varyform commented Apr 8, 2015

We have 3 levels deep nested attributes, so this recursion https://github.com/rails/rails/blob/4-1-stable/actionview/lib/action_view/helpers/translation_helper.rb#L37 is called 45 times x ~2.5ms per input.

@carlosantoniodasilva
Copy link
Member

Interesting @varyform, thanks for the pointer. I think we can definitely speed that up to start with.

@varyform
Copy link

varyform commented Apr 8, 2015

@carlosantoniodasilva forgot to mention important thing: the problem exists in development mode only

@carlosantoniodasilva
Copy link
Member

Hm, so your case might be related to reloading then. How many different locales do you have? (You can check that in the console with I18n.available_locales). Are you using rails-i18n?

@sevenseacat
Copy link

$ cat Gemfile.lock | grep i18n
      i18n (~> 0.7)
      i18n_data (~> 0.6.0)
    i18n (0.7.0)
    i18n_data (0.6.3)
      i18n (>= 0.6.4, <= 0.7.0)

Just one locale in my app, with very few translations. It's basically a brand new app started last week.

@joost
Copy link

joost commented Apr 9, 2015

I have the same issue with a fresh Rails 4.2.1 app (i18n (0.7.0)):
Large table with 180 input boxes (30 'days' x 5 inputs) renders in ~7seconds :S

Adding label: false, placeholder: false, hint: false brings it down to ~700msec.

Limiting locales to only default :en didn't seem to help.

@diegorv
Copy link

diegorv commented May 29, 2015

Same here with Rails 4.2.1 app (i18n (0.7.0)) and Ruby 2.2.2

In production or development

  Rendered client/contracts/_payment_fields.html.erb (39.0ms)
  Rendered client/contracts/_payment_fields.html.erb (39.1ms)
  Rendered client/contracts/_payment_fields.html.erb (40.8ms)
  Rendered client/contracts/_payment_fields.html.erb (42.2ms)
  Rendered client/contracts/_payment_fields.html.erb (39.3ms)
  Rendered client/contracts/_payment_fields.html.erb (39.9ms)
  Rendered client/contracts/_payment_fields.html.erb (59.0ms)
  Rendered client/contracts/_payment_fields.html.erb (62.7ms)
  Rendered client/contracts/_payment_fields.html.erb (46.9ms)
  Rendered client/contracts/_payment_fields.html.erb (52.2ms)
  Rendered client/contracts/_payment_fields.html.erb (48.4ms)
  Rendered client/contracts/_payment_fields.html.erb (68.0ms)
  Rendered client/contracts/_payment_fields.html.erb (51.1ms)
  Rendered client/contracts/_payment_fields.html.erb (60.1ms)
  Rendered shared/theme/_form_div.html.erb (0.7ms)
  Rendered client/contracts/_payment_box.html.erb (739.0ms)

My locale file:

pt-BR:
  activerecord:
    models:
      contract/contract             : "Contrato"
    attributes:
      contract/contract:
        representative_id           : "Representante"
        telemarketing_id            : "Telemarketing"
        number                      : "Numero"
        client_company_id           : "Empresa desse contrato"
        signature_date              : "Data de assinatura"
        observation                 : "Observação"
        items                       : "Itens do contrato"
        price                       : "Valor contrato"
        parcels_number              : "Parcelas"
        first_parcel_date           : "Primeiro vencimento"
        total_payments_price        : ""
        one_contract_payment        : ""

Definitely is a problem with I18n

https://github.com/plataformatec/simple_form/blob/master/lib/simple_form/inputs/base.rb#L172

I remove some lines inside this functions and disable translate_from_namespace from placeholders

      def translate_from_namespace(namespace, default = '')
        model_names = lookup_model_names.dup
        lookups     = []

        lookups << :"#{model_names.join(".")}.#{reflection_or_attribute_name}"
        lookups << default

        t(lookups.shift, scope: :"#{i18n_scope}.#{namespace}", default: lookups).presence
      end

After that

  Rendered client/contracts/_payment_fields.html.erb (14.0ms)
  Rendered client/contracts/_payment_fields.html.erb (11.4ms)
  Rendered client/contracts/_payment_fields.html.erb (12.5ms)
  Rendered client/contracts/_payment_fields.html.erb (12.5ms)
  Rendered client/contracts/_payment_fields.html.erb (12.1ms)
  Rendered client/contracts/_payment_fields.html.erb (12.7ms)
  Rendered client/contracts/_payment_fields.html.erb (13.0ms)
  Rendered client/contracts/_payment_fields.html.erb (12.2ms)
  Rendered client/contracts/_payment_fields.html.erb (12.0ms)
  Rendered client/contracts/_payment_fields.html.erb (13.1ms)
  Rendered client/contracts/_payment_fields.html.erb (12.2ms)
  Rendered client/contracts/_payment_fields.html.erb (12.5ms)
  Rendered client/contracts/_payment_fields.html.erb (13.3ms)
  Rendered client/contracts/_payment_fields.html.erb (12.3ms)
  Rendered shared/theme/_form_div.html.erb (0.3ms)
  Rendered client/contracts/_payment_box.html.erb (188.5ms)

I love SimpleForm, I'm sure you will find a good solution.

@mib32
Copy link

mib32 commented May 29, 2015

In my case form is rendered for like 10-20 seconds in development, in production the time is ok. Profiler points to i18n also

@panmari
Copy link

panmari commented Jun 3, 2015

I sped up the rendering of my form almost 3 times by adding these defaults:

simple_form_for @model, defaults: { placeholder: false, hint: false }

What happened that this is suddenly so slow? A change in rails 4.2.1? The form rendered much faster before.

panmari added a commit to swissdrg/webgrouper that referenced this issue Jun 3, 2015
See heartcombo/simple_form#1227 for some
more details.

Also added slim and started translating some templates.
@akaspick
Copy link

I finally got around to profiling my app after thinking Rails 4.2 was the culprit of my horrendously slow form load times, but finally narrowed it down to simple form. If I use simple_form < 3.1 then my issues go away, but using 3.1, the translation issue slows everything to a crawl.

diegorv's solution works for me for now though.

stayhero added a commit to stayhero/simple_form that referenced this issue Jul 9, 2015
SimpleForm looks up for translation in a huge amount of places which
slows it down when you use nested forms.

See issue: heartcombo#1227

Removed the nested lookups code (including the while-loop) completely as
suggested by @diegorv.
@PragTob
Copy link

PragTob commented Jul 9, 2015

Same problem here - rendering ~20 forms with ~8 form fields each leaves me at 10 seconds + rendering time (in development mode) , setting the defaults: defaults: { placeholder: false, hint: false, label: false } gets that down to 250ms. If I keep just the labels, it takes 3 seconds.

@josevalim
Copy link
Contributor

Thanks for all the info so far folks. On another thread, people were reporting that removing "web-console" was removing the slow down. Can you please verify you achieve good performance by temporarily removing it?

@diegorv can you please send a PR with your changes? You don't need to make the tests pass, I would just like a diff of the changes you have done to help debug this further.

Thanks everyone!

@maxwell
Copy link
Author

maxwell commented Jul 9, 2015

FWIW, my project does not use web-console.

@josevalim
Copy link
Contributor

Thanks @maxwell! Do you have binding_of_caller by any chance in your lock file? Do you use better errors? If you do, can you please try disabling all of them? See issue #1237 for more info.

@uhlenbrock
Copy link

@josevalim We were able to realize a significant speedup in production by removing the placeholder and hint options from the wrapper config initializer. We don't use web-console, binding_of_caller or better errors.

@josevalim
Copy link
Contributor

Thanks @uhlenbrock. Yes, there is something that simple form depends on that became suddenly slower, it may be the I18n plugin or a dependency monkey patching something. I am trying to collect more information before I dive into this. :)

@maxwell
Copy link
Author

maxwell commented Jul 9, 2015

Yes, I do use that. :). Sorry, should have used my brain.

I will disable and get back to you

  • maxwell

On Jul 9, 2015, at 1:48 PM, José Valim notifications@github.com wrote:

Thanks @maxwell! Do you have binding_of_caller by any chance in your lock file? Do you use better errors? If you do, can you please try disabling all of them? See issue #1237 for more info.


Reply to this email directly or view it on GitHub.

@diegorv
Copy link

diegorv commented Jul 10, 2015

done @josevalim :)

@akaspick
Copy link

Removing web-console does improve things somewhat, but it's still not as fast as the recommendation by diegorv (which still allows the web-console gem).

The main issue is that a lot of translation lookups are simply failing. I generated a flamegraph around a single input and had approximately almost 20 failed lookups; all those failed lookups add up to A LOT of wasted rendering time with multiple inputs.

@mib32
Copy link

mib32 commented Jul 10, 2015

@uhlenbrock Could you please tell if you use byebug gem?

@PragTob
Copy link

PragTob commented Jul 10, 2015

Okay so for me removing the 'web-console' gem and by extension its dependencies binding_of_caller and debug_inspector takes me from 3700ms to 330ms view rendering time. I still have byebug installed, though :)

Thanks for the feedback and help!

@uhlenbrock
Copy link

@mib32 No, I do not.

@cipater
Copy link

cipater commented Jul 17, 2015

Tossing my experience with the issue to the mix. I'm running Rails 4.2.1 on Ruby 2.1.1 with SimpleForm 3.1.0.

I landed on this issue because like some others here, I had a form with 8 inputs that was taking 5 to 6 seconds to render. I placed debug statements to try and figure out what was going on only to have the render times increase up to 9 seconds through my testing.

I don't use web-console, but I do use better_errors. Trying the suggestions here and from #1237, I disabled better_errors and byebug and restarted my development server. Lo and behold, my form was rendering in 170ms. Great!

But I was suspicious.

I reenabled those gems, restarted, and the form was still rendering in 170ms. And the rendering time remained stable through a dozen or so page refreshes. However, when I added binding.pry to the page and started refreshing and immediately exiting out of the binding.pry session, the render times were going up by 200 to 400ms with each refresh. Killing and restarting the server immediately fixed the render times.

I haven't profiled anything, but I'm guessing that calling binding.pry is creating some sort of memory leak.

Additionally, following the suggestion to add hint: false to the form reduced the render time from 170ms to 50ms.

For what it's worth, I brought the render times back up to 5 seconds and ran I18n.t within the binding.pry session and it was instantaneous.

@awong-dev
Copy link

@josevalim I will corroborate the conclusions of others on this thread that i18n MissingTranslations is the culprit.

I think the binding_of_caller "slowdown" is a magnification of an underlying issues.

Running a reduced version of my view through ruby-prof, the number of calls to I18n::Backend::Base#translate goes from 98 to 6281 when I add in a simple_fields_for call which renders a partial. The partial in question only has ~200 inputs which should result in O(6000x) more calls to translate. There is some sort of non-linear behavior going on in simple_fields_for. I think it's related to interactions with I18n::MissingTranslation in SimpleForms. If you add the following monkey patch to config/initializers somewhere:

module ActionView
  module Helpers
    module TranslationHelper
      def translate(key, options={})
        I18n.translate(key)
      end
      alias :t :translate
    end
  end
end

The raises of I18n::MissingTranslation disappear from the profile and performance returns (I'm dropping from 4s/page load down to 700ms).

I think the reason binding_of_caller is causing a drastic slowdown is because ActionView::Helpers::TranslationHelper#translate raises for a unknown translation key. For each raise (of which we now have thousands), better_errors calls BetterErrors::ExceptionExtension#set_backtrace which invokes BindingOfCaller::BindingExtensions#callers. This, at minimum, done some array allocation/deallocation in addition to a stack walk.

Just to be completely thorough, I wanted to verify that MRI2's stack walk code wasn't somehow causing a huge slowdown (it shouldn't since it's some very straightforward pointer math). To do this, I downloaded binding_of_caller and stubbed out BindingOfCaller::BindingExtensions#callers to just return an empty array. Things did not get appreciably faster in this case.

Hope this helps!

@josevalim
Copy link
Contributor

Thank you for the overview. Maybe a fix is to just use I18n.translate instead of translate or t in simple form? Do we gain anything by using the latter ones?

@josevalim
Copy link
Contributor

@awong-dev thank you for the detailed wrap-up! I have done two changes to master:

  1. I have removed _html translation support (if you rely on that, your app will break). Explicitly pass the string instead
  2. I am no longer falling back to the view translation helper and using I18n.t directly

Folks on this thread, can you please try master out? Does it fix the performance issues you have seen so far?

@mib32
Copy link

mib32 commented Sep 7, 2015

@josevalim Works for me 👍

@diegorv
Copy link

diegorv commented Sep 7, 2015

@josevalim Works for me too! 👍

@awong-dev
Copy link

@josevalim Thanks for the fast response!

I'm not super familiar with the i18n features in rails, but reading through the docs in http://api.rubyonrails.org/classes/ActionView/Helpers/TranslationHelper.html, I'm wondering if 6ba68fc constitutes an abstraction break.

At first glance, ActionView::Helpers::TranslationHelper#translate seems to be inserting an API layer for rails apps to override the translate call without having to do something crazy like monkeypatching I18n itself. I'm not at a computer today so it's hard for me to do much research, but my worry is there still isn't an explanation for why SimpleForms was generating so danged many calls to tranlsate. The propose patch prevents the bad interaction with binding_of_caller, but I'm guessing it doesn't resolve how come we had so many calls in the first place.

@varyform
Copy link

varyform commented Sep 8, 2015

👍 works!

@josevalim
Copy link
Contributor

3.2.0 released with the performance fix. Thanks everyone!

@panmari
Copy link

panmari commented Sep 22, 2015

👍

@akaspick
Copy link

Works nicely for me as well. 👍

dan-palmer pushed a commit to hmcts/et-pet-et1 that referenced this issue Nov 2, 2015
Upgrades SimpleForm from 3.1.0rc2 to 3.20.

A noticeable performance decrease was spotted when rendering views in
the app after updating to Rails from 4.2.0 to 4.2.4 - in most cases this
was increased running times by 4-5x! It appears as though the issues lie with
I18n::MissingTranslation's in SimpleForm - something that has been addressed
in a later version of the gem.

Please see: heartcombo/simple_form#1227 for
more info.

Updating SimpleForm required a public `has_attribute?` method to be introduced
to all form objects.
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