pathname: eagerly initialize lazy instance variables#22362
Merged
Conversation
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.
Contributor
There was a problem hiding this comment.
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
EagerInitializeExtensionmodule that defines the relevant memoized ivars ininitialize. - Require and
prependthe new extension intoPathnameso 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.
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.
MikeMcQuaid
approved these changes
May 21, 2026
Member
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Thanks! One comment and then can self-merge without rereview.
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.
8b26ad3 to
9bc2ff4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Continues d57efd9 by initialising the lazy memoised instance variables defined in
extend/pathname.rbandDiskUsageExtension(@magic_number,@file_type,@zipinfo,@which_install_info,@disk_usage,@file_count) up front via a newEagerInitializeExtensionmodule that's prepended at class-body time (same shape as the precedent inextend/os/mac/unpack_strategy/zip.rb).Every
Pathnameinstance now shares the same object shape regardless of which lazy method is hit first, so we no longer see:Verified locally: 50
Pathnameinstances exercising the lazy methods in varied orders all collapse to a singleObjectSpace.dumpshape (vs 8+ before). Apathname_spec.rbexample asserts every lazy ivar is defined andnilimmediately afterPathname.newso future regressions are caught in CI.brew lgtm(style, typechecking and tests) with your changes locally?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 theprependprecedent inextend/os/mac/unpack_strategy/zip.rb; ranbrew lgtmlocally; confirmed the warning no longer reproduces and thatObjectSpace.dumpcollapses to a single shape across 50 instances exercising the lazy methods in varied orders.