Skip to content

download_queue: wake on completed downloads#23107

Merged
MikeMcQuaid merged 1 commit into
mainfrom
download-queue-wakeups
Jul 14, 2026
Merged

download_queue: wake on completed downloads#23107
MikeMcQuaid merged 1 commit into
mainfrom
download-queue-wakeups

Conversation

@MikeMcQuaid

Copy link
Copy Markdown
Member
  • The parallel fetch loop paused a fixed 50ms per iteration, even when every download had already finished: Kernel#sleep was 27% of wall samples (~100ms) in brew prof --stackprof fetch for an already-cached formula, the largest single frame ahead of Kernel#require.
  • Warm-cache downloads resolve in microseconds, so almost all of that sleeping was wasted on every brew fetch, brew install and brew upgrade, interactive or scripted, and even the final iteration slept after the last download had finished.
  • Warm-cache brew fetch of a cached formula drops from 0.63-0.70s to 0.45-0.52s (~200ms, ~25-30%), measured both on a TTY and piped, and Kernel#sleep disappears from its profile entirely.
  • Completed downloads are reported as they finish instead of on the next 50ms poll tick.
  • Skip the end-of-iteration pause entirely when the partition left no downloads pending; previously even the final iteration slept.
  • Wait on a Concurrent::Event signalled by an on_resolution! callback on every download future instead of sleeping, so warm caches never pay a poll interval.
  • On animated TTYs the wait times out after 50ms, purely to keep redrawing the spinner and progress bars at a steady cadence while downloads are in flight; otherwise after 1s as a safety net.
  • The event is reset before each wait and the remaining futures are re-checked in between, so a download resolving between the partition and the reset cannot cause a lost wakeup.

  • Have you followed our Contributing guidelines?
  • Have you checked for other open Pull Requests for the same change?
  • Have you explained what your changes do? Performance claims (e.g. "this is faster") must include Hyperfine benchmarks.
  • Have you explained why you'd like these changes included, not just what they do?
  • For bug fixes, have you given step-by-step brew commands to reproduce the bug?
  • Have you written new tests (excluding integration tests)? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) locally?

  • AI was used to generate or assist with generating this PR.

Anthropic Fable 5 max with local review and testing.


- The parallel fetch loop paused a fixed 50ms per iteration, even
  when every download had already finished: `Kernel#sleep` was 27%
  of wall samples (~100ms) in `brew prof --stackprof fetch` for an
  already-cached formula, the largest single frame ahead of
  `Kernel#require`.
- Warm-cache downloads resolve in microseconds, so almost all of
  that sleeping was wasted on every `brew fetch`, `brew install`
  and `brew upgrade`, interactive or scripted, and even the final
  iteration slept after the last download had finished.
- Warm-cache `brew fetch` of a cached formula drops from
  0.63-0.70s to 0.45-0.52s (~200ms, ~25-30%), measured both on a
  TTY and piped, and `Kernel#sleep` disappears from its profile
  entirely.
- Completed downloads are reported as they finish instead of on
  the next 50ms poll tick.
- Skip the end-of-iteration pause entirely when the partition left
  no downloads pending; previously even the final iteration slept.
- Wait on a `Concurrent::Event` signalled by an `on_resolution!`
  callback on every download future instead of sleeping, so warm
  caches never pay a poll interval.
- On animated TTYs the wait times out after 50ms, purely to keep
  redrawing the spinner and progress bars at a steady cadence
  while downloads are in flight; otherwise after 1s as a safety
  net.
- The event is reset before each wait and the remaining futures
  are re-checked in between, so a download resolving between the
  partition and the reset cannot cause a lost wakeup.
Copilot AI review requested due to automatic review settings July 14, 2026 16:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves Homebrew::DownloadQueue’s parallel fetch loop by replacing a fixed poll-and-sleep cadence with an event-driven wakeup when any download future resolves, eliminating wasted sleep time in warm-cache scenarios and reporting completions sooner.

Changes:

  • Replace the fixed sleep 0.05 polling loop with a Concurrent::Event signaled by Future#on_resolution!, waking immediately on completion.
  • Preserve spinner/progress redraw cadence on TTYs via a short wait timeout, while using a longer timeout for non-TTY output.
  • Add an RSpec example asserting the queue no longer calls sleep in the parallel fetch path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
Library/Homebrew/download_queue.rb Switches the parallel fetch loop from fixed-interval sleeping to an event-driven wait signaled by download future resolution.
Library/Homebrew/test/download_queue_spec.rb Adds a spec ensuring the queue wakes on completion without calling sleep (non-TTY mode).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@MikeMcQuaid MikeMcQuaid enabled auto-merge July 14, 2026 18:04
@MikeMcQuaid MikeMcQuaid added this pull request to the merge queue Jul 14, 2026
Merged via the queue into main with commit 22f8b50 Jul 14, 2026
45 checks passed
@MikeMcQuaid MikeMcQuaid deleted the download-queue-wakeups branch July 14, 2026 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants