Skip to content

pathname: eagerly initialize lazy instance variables#22362

Merged
p-linnane merged 3 commits into
mainfrom
pathname-shape-variations
May 21, 2026
Merged

pathname: eagerly initialize lazy instance variables#22362
p-linnane merged 3 commits into
mainfrom
pathname-shape-variations

Conversation

@p-linnane
Copy link
Copy Markdown
Member

@p-linnane p-linnane commented May 21, 2026

Continues d57efd9 by initialising the lazy memoised instance variables defined in extend/pathname.rb and DiskUsageExtension (@magic_number, @file_type, @zipinfo, @which_install_info, @disk_usage, @file_count) up front via a new EagerInitializeExtension module that's prepended at class-body time (same shape as the precedent in extend/os/mac/unpack_strategy/zip.rb).

Every Pathname instance now shares the same object shape regardless of which lazy method is hit first, so we no longer see:

warning: The class Pathname reached 8 shape variations, instance variables accesses will be slower and memory usage increased.
It is recommended to define instance variables in a consistent order, for instance by eagerly defining them all in the #initialize method.

Verified locally: 50 Pathname instances exercising the lazy methods in varied orders all collapse to a single ObjectSpace.dump shape (vs 8+ before). A pathname_spec.rb example asserts every lazy ivar is defined and nil immediately after Pathname.new so future regressions are caught in CI.


  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them? Performance claims (e.g. "this is faster") must include Hyperfine benchmarks.
  • Have you written new tests (excluding integration tests) for your changes? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) with your changes locally?

  • AI was used to generate or assist with generating this PR. Please specify below how you used AI to help you, and what steps you have taken to manually verify the changes. Non-maintainers may only have one AI-assisted/generated PR open at a time.

Claude Code (Opus 4.7) located the remaining lazy ivars, drafted the prepended initializer module, and ran the verification steps. Manually reviewed the diff against the conventions in extend/pathname/ (file layout, naming, requires_ancestor, sig style) and the prepend precedent in extend/os/mac/unpack_strategy/zip.rb; ran brew lgtm locally; confirmed the warning no longer reproduces and that ObjectSpace.dump collapses to a single shape across 50 instances exercising the lazy methods in varied orders.


Continues d57efd9 by initialising the lazy memoised instance
variables defined in `extend/pathname.rb` and `DiskUsageExtension`
(`@magic_number`, `@file_type`, `@zipinfo`, `@which_install_info`,
`@disk_usage`, `@file_count`) up front via a new prepended
`EagerInitializeExtension` module, so every `Pathname` instance
shares the same object shape and we no longer hit:

    warning: The class Pathname reached 8 shape variations,
    instance variables accesses will be slower and memory usage
    increased.
Copilot AI review requested due to automatic review settings May 21, 2026 04:19
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 addresses Ruby “object shape variation” warnings for Pathname by eagerly defining previously-lazy memoized instance variables up front, ensuring all Pathname instances share a consistent instance-variable layout regardless of which memoized method is called first.

Changes:

  • Add a new EagerInitializeExtension module that defines the relevant memoized ivars in initialize.
  • Require and prepend the new extension into Pathname so initialization happens for all instances.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
Library/Homebrew/extend/pathname/eager_initialize_extension.rb New prepended extension to eagerly initialize previously-lazy Pathname/DiskUsageExtension ivars.
Library/Homebrew/extend/pathname.rb Requires the new extension and prepends it into Pathname at class-body time.

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

Comment thread Library/Homebrew/extend/pathname.rb
Lock the eager-init invariant so adding a new lazy memoised ivar to
`Pathname`/`DiskUsageExtension` without updating
`EagerInitializeExtension` (and thus reintroducing shape churn) is
caught in CI.
Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Thanks! One comment and then can self-merge without rereview.

Comment thread Library/Homebrew/extend/pathname/eager_initialize_extension.rb
Explain why the module exists (shape cache warning) and the
contract for adding/removing lazy memoised ivars on `Pathname`
or its mixed-in extensions, per review feedback.
@p-linnane p-linnane force-pushed the pathname-shape-variations branch from 8b26ad3 to 9bc2ff4 Compare May 21, 2026 15:19
@p-linnane p-linnane enabled auto-merge May 21, 2026 15:19
@p-linnane p-linnane added this pull request to the merge queue May 21, 2026
Merged via the queue into main with commit d8deaca May 21, 2026
36 checks passed
@p-linnane p-linnane deleted the pathname-shape-variations branch May 21, 2026 16:15
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