brew icon indicating copy to clipboard operation
brew copied to clipboard

Profile ActiveSupport `require` time and investigate removal or reduction

Open MikeMcQuaid opened this issue 4 years ago • 7 comments
trafficstars

Provide a detailed description of the proposed feature

ActiveSupport is something we use fairly minimally but, I speculate, adds a lot to our require "global.rb" time. We should investigate whether this is actually the case (e.g. by using brew prof --stackprof help) and, if so, try to mess around with the vendored files to see if we can e.g. remove concurrent ruby, i18n, etc. support while still using the stuff we care about (e.g. .present?)

CC @Bo98 for more thoughts as he'd looked at this before.

What is the motivation for the feature?

Speeding up the brew process boot time to make all Ruby commands faster.

How will the feature be relevant to at least 90% of Homebrew users?

Everyone would like a faster brew.

What alternatives to the feature have been considered?

Shipping HOMEBREW_BOOTSNAP to all users by default if it provides enough speedup.

MikeMcQuaid avatar Feb 03 '21 16:02 MikeMcQuaid

I will do some measurements. I did have some require shims to look at individual require times quite closely and I'll set that up again.

Bo98 avatar Feb 03 '21 16:02 Bo98

Thanks @Bo98.

MikeMcQuaid avatar Feb 03 '21 17:02 MikeMcQuaid

I’m not experienced with profiling ruby code, but I took a shot at this anyway. It looks like time.rb is by far the slowest ActiveSupport class to load, taking almost 30% of the samples for brew help. The whole command took about 1s to run on my machine, and removing time.rb only reduced it bv ~.1s.

zachauten avatar Sep 14 '21 20:09 zachauten

Yeah that one is quite slow due to tzinfo. Given it's only used in cleanup.rb, which is a file not loaded by default, and a test afaik (and maybe some taps), it can probably be moved or removed relatively easily with tap compatibility being the main concern.

You do sometimes needs to watch out with some of the timings. os/mac/version appears to be one of the slowest things to load, but it just because it's one of the first things to load and starts picking up a lot of things for the first time, like require "utils".

I have now found what I did before when looking at Homebrew-specific files. I replaced every internal require with homebrew_require (so it wouldn't cover direct gem requires in global but would cover most other things) and monitored that, creating a flamegraph type output. I'll try fix that up again.

The last experiment did result in changes. Most notably #8382.

Bo98 avatar Sep 15 '21 01:09 Bo98

Yeah that one is quite slow due to tzinfo. Given it's only used in cleanup.rb, which is a file not loaded by default, and a test afaik (and maybe some taps), it can probably be moved or removed relatively easily with tap compatibility being the main concern.

šŸ‘šŸ»


Honestly, I'm fairly tempted to just reimplement the stuff we care about e.g. blank?. ActiveSupport has some nice implementations here but naive ones are easy to implement with fewer dependencies and will speed up loading for us.

MikeMcQuaid avatar Sep 15 '21 11:09 MikeMcQuaid

Honestly, I'm fairly tempted to just reimplement the stuff we care about e.g. blank?. ActiveSupport has some nice implementations here but naive ones are easy to implement with fewer dependencies and will speed up loading for us.

Even reducing dependency count irregardless of speed is a good enough reason for me (see: mechanize), though ActiveSupport is not bad in regards for that.

Honestly the best way to approach this is just one by one. numeric/time has been identified as the biggest individual contributor, and is also probably the easiest to eliminate - others may be harder like core_ext/string/inflections and may or may not be worth it (0.03s) if you compare it with whatever an alternative implementation might be.

Bo98 avatar Sep 15 '21 11:09 Bo98

Honestly the best way to approach this is just one by one. numeric/time has been identified as the biggest individual contributor, and is also probably the easiest to eliminate - others may be harder like core_ext/string/inflections and may or may not be worth it (0.03s) if you compare it with whatever an alternative implementation might be.

šŸ‘šŸ»

MikeMcQuaid avatar Sep 15 '21 11:09 MikeMcQuaid

@dduugg Do you think there's much more here or do you think it's worth just closing this out?

MikeMcQuaid avatar Feb 22 '23 16:02 MikeMcQuaid

Hi, I think we can cut string/inflections too. Most of the time there is actually spent loading i18n, which we might only need in one place (manpages.rb), does that seem right? Additionally, I think pluralize might be the only inflection that's used. If that's the case we can a) write a small util to handle that logic ourselves, b) move the require into the files that directly use pluralize, c) avoid the problem by writing things like formula(e), is/are, etc..

Which approach makes the most sense to you?

dduugg avatar Feb 22 '23 18:02 dduugg

Hi, I think we can cut string/inflections too. Most of the time there is actually spent loading i18n, which we might only need in one place (manpages.rb), does that seem right?

Yeh, sounds right šŸ‘šŸ»

Additionally, I think pluralize might be the only inflection that's used. If that's the case we can a) write a small util to handle that logic ourselves, b) move the require into the files that directly use pluralize, c) avoid the problem by writing things like formula(e), is/are, etc..

Agreed, think we can maintain that/something ourselves šŸ‘šŸ»

MikeMcQuaid avatar Feb 22 '23 19:02 MikeMcQuaid