upgrade: parallelize bottle tab fetching#22118
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to speed up brew upgrade startup and provide visible progress by fetching bottle tab manifests concurrently instead of sequentially per formula, reducing perceived “hang time” when many formulae are upgraded.
Changes:
- Introduces a shared
Homebrew::DownloadQueueto enqueue bottle tab manifest downloads for all upgrade targets. - Fetches the queued bottle tab downloads in parallel, then proceeds with installer filtering/selection.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| installers.filter_map do |fi| | ||
| all_runtime_deps_installed = fi.bottle_tab_runtime_dependencies.presence&.all? do |dependency, hash| | ||
| minimum_version = if (version = hash["version"]) | ||
| Version.new(version) | ||
| end | ||
| Dependency.new(dependency).installed?(minimum_version:, minimum_revision: hash["revision"].to_i) | ||
| end |
There was a problem hiding this comment.
bottle_tab_runtime_dependencies is never populated in this method (it’s only set in FormulaInstaller#prelude), so all_runtime_deps_installed will always be nil here and this dependent-skipping logic will never trigger. Consider deriving the runtime deps directly from fi.formula.bottle_tab_attributes["runtime_dependencies"] after the queue fetch, or otherwise populating bottle_tab_runtime_dependencies before this check (without running the full prelude).
There was a problem hiding this comment.
Yeah this is a valid point and a pre-existing issue.
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Nice idea and work so far! This makes sense broadly but I think a few changes would help:
- should be done for install/reinstall/upgrade
- should be in an existing download queue if possible so the parallelisation can occur with other downloads (this may not be possible! if so: please explain why both here and in a Ruby comment)
This is already being done for install 1 and reinstall 2 via
I don't think that works cleanly, because here in brew/Library/Homebrew/upgrade.rb Lines 91 to 102 in 4a95077 And this logic is specific to upgrade (although it isn't really working as expected due to the issue Copilot pointed out above 😭). Footnotes |
Currently, `FormulaInstaller#fetch_bottle_tab` is called sequentially and silently in the background for each formula to upgrade. When there is a large number of formulae to upgrade, this can lead to a significant delay before the any output is shown to the user. Instead, let's parallelize the bottle tab fetching. This makes the upgrade progress much faster, and also visibly shows the user that something is happening. A naive and rough benchmark involving 8 outdated formulae shows that this shortens `Upgrade#formula_installers` from ~4.7s to ~2.1s. With more outdated formulae, this difference should become even more significant.
bce0d1a to
6498306
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fix the current situation where `fi.bottle_tab_runtime_dependencies` is never populated, which effectively makes the runtime dependency check for skipping unnecessary upgrades a no-op.
6498306 to
ca85c3f
Compare
Currently,
FormulaInstaller#fetch_bottle_tabis called sequentially and silently in the background for each formula to upgrade. When there is a large number of formulae to upgrade, this can lead to a significant delay before the any output is shown to the user.Instead, let's parallelize the bottle tab fetching. This makes the upgrade progress much faster, and also visibly shows the user that something is happening.
A naive and rough benchmark involving 8 outdated formulae shows that this shortens
Upgrade#formula_installersfrom ~4.7s to ~2.1s. With more outdated formulae, this difference should become even more significant.brew lgtm(style, typechecking and tests) with your changes locally?