I tested it in development and production modes but generated flamgraphs come from development mode
Rails log give me these speed up

• X 5.0 for development
• X 1.78 for production ( edit: not reproduced afterwards )

I use the default configuration for cache_discovery.
config/initializers/simple_form.rb

...
  # Cache SimpleForm inputs discovery
  # config.cache_discovery = !Rails.env.development?
...

In development cache_discovery is off

[3] pry(main)> Rails.env
=> "development"
[4] pry(main)> SimpleForm.cache_discovery
=> false

In production cache_discovery is on

irb(main):001:0> Rails.env
=> "production"
irb(main):002:0> SimpleForm.cache_discovery 
=> true

I will generate 2 flamegraphs without and with the fix in production mode

Hello,

In production mode with and without the patch of this PR:

• the rendering durations are identical,
• the generated flamgraphs are identical.

I can no longer reproduce my performance improvement in production with only this patch. Which seems logical compared to what you say. It must have been due to another correction of my view (Oops).

So this improvement is only valid for the development mode.

Thanks @kriom, I believe that in production that discovery cache option takes place and helps prevent the many NameError rescues you were seeing in development.

I hope to circle back on this for review in the next few days.

@carlosantoniodasilva I came across this issue while researching a similar simple_form rendering time issue and I found in our configuration, we had the default generated config that has the discovery cache option (that turns it off in non-development) commented out. Would a PR to change that so the option is not commented out in the configuration file be accepted?

I suspect many people are running simple_form in production with cache_discovery enabled and are not aware. The change I suggest above won't help them but at least new users won't experience that.

I was also surprised that I couldn't find any documentation for the discovery cache configuration setting. Maybe the impact isn't as dramatic in production but when I toggled it on in development, the admittedly complex form we are rendering took a bit less than 50% of the original time to render.

Thanks for your work on this one @kriom, and my apologies for not circling back on it back then. We got a similar change in #1810, so I went ahead and merged that, credited both of you in the changelog.

@cymen I'm afraid that enabling that by default could have side-effects with code reloading in dev, i.e. if you make changes to input code it could remove the previous cached constant for example... this is off the top of my head, untested, but sounds like it would be possible. If you would like to give that a try and doesn't run into any issues, we could consider enabling that on by default in dev, but otherwise I think this should be fine as is. We could of course improve documentation on that config. (it should be on by default in production though.)

Thanks all!

I'm reverting the merged 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.