heartcombo / simple_form

Forms made easy for Rails! It's tied to a simple DSL, with no opinion on markup.
http://blog.plataformatec.com.br/tag/simple_form
MIT License
8.21k stars 1.31k forks source link

Speed up nested #simple_fields_for usage by a considerable amount #1810

Closed meanphil closed 1 year ago

meanphil commented 1 year ago

The FormBuilder#find_mapping method is relying on Object#const_get raising an exception if the constant doesn't exist, rather than directly checking if it exists with Object#const_defined?.

Exceptions are slow due to generating backtraces - and then when other debugging gems (such as did_you mean, which are included with Ruby and Rails these days) get involved it resulted in excessive Object allocation.

Normally the discovery_cache would mitigate this, so there wouldn't be many calls to #mapping_override, however when you use #simple_fields_for with a collection/has_many association the cache is empty for each item in the collection.

Before this change this code was taking approximately 2 seconds and 2.9million allocations for 235 records:

<%= f.simple_fields_for :provider_access_masks, @access_masks do |pam| %>
  <%= pam.input :id, as: 'hidden' %>
  <%= pam.input :_destroy, as: 'boolean', label: "Delete", class: 'destroy' %>
<% end %>

After this change it was down to approximately 100ms and 200,000 allocations. (All on my M1 MacBook Air)

carlosantoniodasilva commented 3 months ago

I'm reverting this change, as it caused a regression. See #1824 for more info.

Happy to review something else that fixes the original problem without the regression, but for now we'll have to live with it.