Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.08%. Comparing base (01f1430) to head (8b797de).
⚠️ Report is 16 commits behind head on master.

@@           Coverage Diff           @@
##           master    #8782   +/-   ##
=======================================
  Coverage   99.08%   99.08%           
=======================================
  Files         139      139           
  Lines        4043     4045    +2     
=======================================
+ Hits         4006     4008    +2     
  Misses         37       37           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Thank you for this PR!

I'm not sure if Copilot's suggestions are applicable here, and I don't have enough insight to provide a thorough review.

I'll leave to others with more expertise to review and make the merging decision

@tagliala I've just commented on Copilot's suggestions. They don't make sense.

Can you tag people who could potentially review the PR?

They don't make sense.

Strict but fair. I did also downvote the response and added the explanation, hoping that this would help

Can you tag people who could potentially review the PR?

Done

@alexsmartens thank you. I’d like to see what the problem is first that is caused by not having this. It sounds serious so I wonder why we haven’t seen it reported before. We’ve done something similar so it’s not surprising to add something like this but it would be helpful to know and see what the issue is that it’s addressing. We generate dev and test apps locally in this repo but my guess it’s better to see this in an actual app so you may want to demonstrate the issue with our demo app which lives in its own repository.

@javierjulio I've been looking into optimizing my app boot up time for dev/test environments. The boot profiling indicated that most of the booting time comes from loading gems. active_admin was one of the slowest gems.

I tried to find any commonalities between the slow gems and one commonality is premature loading of ActiveRecord. Rails Guideline on this topic:

Rails is responsible for the load order of these frameworks, so when you load frameworks, such as ActiveRecord::Base, prematurely you are violating an implicit contract your application has with Rails. Moreover, by loading code such as ActiveRecord::Base on boot of your application you are loading entire frameworks which may slow down your boot time and could cause conflicts with load order and boot of your application.

That's when I identified the root cause of the issue and created this PR. In my app, this change reduced active_admin loading time from 755ms to 655ms

For reference, active_admin is still the slowest loading gem. I'll be happy to push up more changes if I do further optimizations

Hello,

sorry for the silence (but as I already mentioned I cannot do to much on this)

So this is a performance improvement rather than a bug fix and it also may allow us to fix an issue (that I can't replicate) when I use activeadmin together with an Active Record adapter different than the default one.

In fact, under some circumstances, we've seeing ActiveAdmin::DatabaseHitDuringLoad error. But again, I'm not able to replicate it (ifad/chronomodel#349)

I was trying to experiment myself against this branch to see if there are improvements. How do you measure Rails boot time?

I'm trying with time rails runner 'puts "hello"' but I cannot find differences on a small application

hi there,
just checking in, any plans to merge this?

@alexsmartens no because you haven't replied with what we asked for. We'd like to be able to try this for ourselves to vet the change. Thank you.

Is there anything else other than #8782 (comment)?

Hello again,

I would like to test the performance improvements by myself. As I mentioned in my previous comment, I'm not familiar with benchmarks of rails boot, and my attempts to compare results between master and this branch were not successful (no visible gain on a small application)