Skip to content

pathname: build eager-init type objects once, not per allocation#23267

Merged
MikeMcQuaid merged 2 commits into
Homebrew:mainfrom
hyuraku:pathname-eager-init-type-alias
Jul 22, 2026
Merged

pathname: build eager-init type objects once, not per allocation#23267
MikeMcQuaid merged 2 commits into
Homebrew:mainfrom
hyuraku:pathname-eager-init-type-alias

Conversation

@hyuraku

@hyuraku hyuraku commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

By default, runtime type checking is off. In that mode, standalone/sorbet.rb
replaces T.let with a version that ignores the type argument. But Ruby still
evaluates that argument first. So T.let(nil, T.nilable(String)) builds a type
object and then throws it away. EagerInitializeExtension#initialize runs every
time a Pathname is created, and it does this six times. #23078 found this was
one of the biggest remaining CPU costs in brew upgrade --dry-run and
brew outdated.

This change moves the three types into T.type_alias constants. They are built
once, and #initialize just uses the constants. The types themselves do not
change, so Sorbet still sees @magic_number as T.nilable(String) and so on.

This change does not touch sorbet-runtime itself. #23078 tried to make
T.nilable, T::Array[] and friends share instances globally, but that broke
how T::Props reads optionality and was reverted in #23091. Moving only these
constants avoids that problem.

Behaviour is the same: brew outdated and brew info --installed --json=v2
print byte-identical output before and after. With the runtime disabled,
Hyperfine shows a consistent speedup (three alternating rounds of 20 runs,
3 warmup, main vs this branch):

Command before (main) after speedup user CPU
brew outdated 1.163 s 1.071 s 1.09× 0.747 s → 0.668 s
brew info --installed --json=v2 1.449 s 1.349 s 1.07× 0.931 s → 0.829 s

The ratio was stable across all three rounds (1.08–1.09 and 1.07–1.08). Commands
that create fewer Pathnames show no measurable change. A stackprof profile of
brew outdated shows the union-type construction dropping from 36 samples to 9.


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

AI (Claude Code) found the cost by profiling brew outdated, wrote the change,
and ran the benchmarks. I reviewed the change and ran brew lgtm locally.

@MikeMcQuaid
MikeMcQuaid requested a review from dduugg July 22, 2026 12:29

@MikeMcQuaid MikeMcQuaid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice cleanup! Interested in @dduugg's thoughts here before we merge.

Also: one test is 🔴

@hyuraku
hyuraku force-pushed the pathname-eager-init-type-alias branch from 90386c3 to c7f5bb0 Compare July 22, 2026 13:16

@dduugg dduugg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Much safer than my approach, ty! It's a nice, resuable technique that we can use in other hotspots as well.

@MikeMcQuaid
MikeMcQuaid added this pull request to the merge queue Jul 22, 2026
@MikeMcQuaid

Copy link
Copy Markdown
Member

Thanks again @hyuraku!

Merged via the queue into Homebrew:main with commit f7a5a2f Jul 22, 2026
40 checks passed
@hyuraku
hyuraku deleted the pathname-eager-init-type-alias branch July 22, 2026 15:30
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