Skip to content

Speed up brew style and brew readall - #23435

Merged
MikeMcQuaid merged 1 commit into
mainfrom
speedup-style-readall
Aug 4, 2026
Merged

Speed up brew style and brew readall#23435
MikeMcQuaid merged 1 commit into
mainfrom
speedup-style-readall

Conversation

@MikeMcQuaid

Copy link
Copy Markdown
Member
  • brew readall: validate tap formulae and casks across forked workers (combination loop inside each worker so the on_system cache keeps its per-file locality) and syntax-check Ruby files in-process with RubyVM::InstructionSequence.compile_file and a Warning buffer instead of spawning ruby -c -w per file.
  • brew style: run shellcheck+shfmt and actionlint on background threads with buffered output while RuboCop runs on the main thread, chunk shellcheck across CPU cores and pass --parallel to RuboCop with --fix (supported since RuboCop 1.41).
  • Resolve linter executables before spawning threads so they cannot race to install formulae.
  • Readall.valid_aliases?: use a single glob and Set lookup rather than one glob per alias.
  • Fix an end indentation warning in cask/cask.rb that made brew readall --syntax fail.
  • Hyperfine benchmarks (18-core Mac, mean of 2 runs, warm RuboCop cache for style runs, all exit codes 0):
    • brew readall homebrew/core: 34.36s -> 4.71s (7.3x faster)
    • brew readall homebrew/cask: 40.69s -> 5.17s (7.9x faster)
    • brew style homebrew/core (warm): 2.93s -> 2.20s
    • brew style homebrew/cask (warm): 2.37s -> 2.18s
    • brew style on Homebrew/brew itself: 13.2s -> 7.8s
    • brew readall --syntax: 18.2s -> 0.5s
  • Tap style runs are RuboCop-bound so barely change; the larger style win is on Homebrew/brew where all four linters run. CI runners with fewer cores should expect roughly 3-4x on readall.

  • 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?

  • I did not use AI/LLM to create this PR, or I disclosed the tool/model below and reviewed its output; I did not attribute commits to AI and will answer maintainer questions and review comments myself without AI/LLM.

Claude Fable 5 max with local review and testing.


- `brew readall`: validate tap formulae and casks across forked
  workers (combination loop inside each worker so the `on_system`
  cache keeps its per-file locality) and syntax-check Ruby files
  in-process with `RubyVM::InstructionSequence.compile_file` and a
  `Warning` buffer instead of spawning `ruby -c -w` per file.
- `brew style`: run shellcheck+shfmt and actionlint on background
  threads with buffered output while RuboCop runs on the main
  thread, chunk shellcheck across CPU cores and pass `--parallel`
  to RuboCop with `--fix` (supported since RuboCop 1.41).
- Resolve linter executables before spawning threads so they
  cannot race to install formulae.
- `Readall.valid_aliases?`: use a single glob and `Set` lookup
  rather than one glob per alias.
- Fix an `end` indentation warning in `cask/cask.rb` that made
  `brew readall --syntax` fail.
- Hyperfine benchmarks (18-core Mac, mean of 2 runs, warm RuboCop
  cache for style runs, all exit codes 0):
  - `brew readall homebrew/core`: 34.36s -> 4.71s (7.3x faster)
  - `brew readall homebrew/cask`: 40.69s -> 5.17s (7.9x faster)
  - `brew style homebrew/core` (warm): 2.93s -> 2.20s
  - `brew style homebrew/cask` (warm): 2.37s -> 2.18s
  - `brew style` on Homebrew/brew itself: 13.2s -> 7.8s
  - `brew readall --syntax`: 18.2s -> 0.5s
- Tap style runs are RuboCop-bound so barely change; the larger
  style win is on Homebrew/brew where all four linters run. CI
  runners with fewer cores should expect roughly 3-4x on readall.
Copilot AI lite review requested due to automatic review settings August 4, 2026 14:16

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 the performance of developer-facing linting/validation commands by parallelising expensive checks and reducing per-file process spawns. It focuses on speeding up brew style (by overlapping RuboCop with other linters) and brew readall (by validating tap contents across forked workers and doing Ruby syntax checks in-process).

Changes:

  • brew style: run shell checks (shellcheck/shfmt) and actionlint concurrently with RuboCop; chunk shellcheck across CPU cores; enable RuboCop --parallel even with --fix.
  • brew readall: validate tap formulae/casks across forked workers; replace ruby -c -w subprocesses with RubyVM::InstructionSequence.compile_file plus captured warnings/errors; speed up alias validation using a single glob + Set.
  • Update tests to cover the new parallel shellcheck chunking and in-process Ruby syntax validation, and adjust mocks to the new system_command-based implementations.

Reviewed changes

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

Show a summary per file
File Description
Library/Homebrew/style.rb Adds threaded execution + buffered output for non-RuboCop linters; implements shellcheck chunking; adjusts RuboCop parallelism and switches shfmt/actionlint to system_command.
Library/Homebrew/readall.rb Adds fork-based worker slicing for tap validation and syntax checks; switches Ruby syntax validation to in-process compilation with warning capture; optimises alias validation.
Library/Homebrew/cmd/readall.rb Passes Pathname objects into Readall.valid_ruby_syntax? for the updated signature/implementation.
Library/Homebrew/extend/os/mac/readall.rb Extends the valid_casks? API to accept an optional files: list, matching the new parallel validation path.
Library/Homebrew/test/style_spec.rb Updates actionlint/shfmt tests to use system_command stubs and adds a new test for parallel shellcheck chunking/merging.
Library/Homebrew/test/cmd/readall_spec.rb Adds tests for in-process Ruby syntax validation and for parallel worker aggregation/validation behaviour.
Library/Homebrew/cask/cask.rb Fixes indentation/structure to avoid a syntax/parse warning that impacted brew readall --syntax.

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

@MikeMcQuaid
MikeMcQuaid added this pull request to the merge queue Aug 4, 2026
Merged via the queue into main with commit a1c4ddb Aug 4, 2026
44 checks passed
@MikeMcQuaid
MikeMcQuaid deleted the speedup-style-readall branch August 4, 2026 16:17
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.

4 participants