perf: add Utils.parallel_map, use it in tap-info and services list#22992
Conversation
813771e to
abdd9b8
Compare
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Thanks! Think this is worth it for tap-info but not for services based on your benchmarks.
Also would be fine if Concurrent is "better" if you wanted to use that inside the helper instead (but could be follow-up)
abdd9b8 to
a0cf4f8
Compare
|
Thanks @MikeMcQuaid! Dropped the |
a0cf4f8 to
8bee91d
Compare
|
Since the last comment: #22989 merged, so its inline-thread section rendering in |
There was a problem hiding this comment.
Pull request overview
This PR introduces a reusable Utils.parallel_map helper to run per-item work concurrently (one thread per item) and applies it to places that were previously doing ad-hoc threading or serial GitHub API waits, notably improving brew tap-info performance while preserving output order.
Changes:
- Add
Utils.parallel_map(items, &block)to run blocking work concurrently and return results in input order. - Use
Utils.parallel_mapinbrew tap-info(no-args stats and--json) to overlap per-tap GitHub API calls and other I/O. - Replace inline thread rendering in
SystemConfig.dump_verbose_configwith the new helper.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Library/Homebrew/utils.rb | Adds Utils.parallel_map threading helper and documentation. |
| Library/Homebrew/test/utils_spec.rb | Adds tests for Utils.parallel_map behavior. |
| Library/Homebrew/system_config.rb | Refactors verbose config section rendering to use Utils.parallel_map. |
| Library/Homebrew/cmd/tap-info.rb | Parallelizes per-tap stats collection and retrofits JSON generation to use Utils.parallel_map. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
8bee91d to
e7277d9
Compare
Summary
In #22975 the per-tap
Tap#to_hashcalls were made concurrent with one plainThreadper tap, and review suggested extracting a single block helper if the approach got reused. This PR adds that helper, applies it to the serial GitHub API loop in barebrew tap-infoand retrofits the inline-thread call sites from #22975 and #22989.Changes
Utils.parallel_map(items, &block): one thread per item, results in input order, first exception re-raised byThread#value(withreport_on_exceptiondisabled so it isn't also printed to stderr). The docstring notes it is only worthwhile for blocking waits, since the GVL serializes Ruby execution.brew tap-info(no arguments): the statistics loop calledTap#private?serially, one GitHub API request per non-core tap, plus per-tap file counting.brew tap-info --installed --json: retrofitted to use the helper; behavior and performance unchanged from perf/tap-info: generate tap hashes concurrently (up to ~77% faster) #22975.SystemConfig.dump_verbose_config: retrofitted the inline-thread section rendering from perf/config: gather system configuration concurrently (~31% faster) #22989 (merged after this PR was opened); behavior and performance unchanged,brew configoutput verified identical.Per review, an earlier
brew services listconversion was dropped: its benchmark was within noise at typical service counts.Output is byte-identical before and after.
Benchmarks (Hyperfine, 10 runs each, warmup 2,
HOMEBREW_SORBET_RUNTIMEunset, 9 taps of which 7 need a GitHub API call):brew tap-infobrewcommands to reproduce the bug?brew lgtm(style, typechecking and tests) locally?Claude Code was used to profile the commands, implement the helper and call sites and write the helper specs (ordering under staggered sleeps, exception propagation). Verified by byte-identical before/after output, interleaved Hyperfine benchmarks on clean trees and running
brew lgtmlocally.