formula: default to cache for Dependency#expand usage#20827
Merged
Conversation
Member
Author
|
Wasn't sure if there was any reasonable use case of Alternatively, could add |
fb17e13 to
ebb2b67
Compare
Dependency#expand usage
This was referenced Oct 7, 2025
MikeMcQuaid
approved these changes
Oct 7, 2025
Member
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Looks fine! When is use_cache: false used?
Member
Author
I don't think it is used in Homebrew/brew. Seeing as it is internal API, may just remove. |
Performance can be very slow without the cache to avoid the duplicate parts of the dependency tree so use the cache by default when running `recursive_dependencies` and `declared_runtime_dependencies`. All other usage of `Dependency#expand` in Homebrew/brew already uses the cache.
ebb2b67 to
cd1defa
Compare
Member
Author
|
Simplified to just required changes as we can add later on if needed. Will likely merge in an hour or so to finish up Qt PR. |
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.
brew stylewith your changes locally?brew typecheckwith your changes locally?brew testswith your changes locally?Performance can be very slow without the cache to prune the duplicate parts of the dependency tree so use the cache by default when running
Formula#recursive_dependencies. All other usage ofDependency#expandin Homebrew/brew already uses the cache.For blocks, I decided to create a temporary cache entry and purge it to avoid unnecessary memory consumption. Used the same naming scheme as FormulaInstaller. Not sure if any chance of collision (if so could add a
SecureRandom.uuidor something else)brew/Library/Homebrew/formula_installer.rb
Line 762 in 7c21e41
This should help with another slow down seen in:
Even on macOS with smaller dependency tree, there is a noticeable impact: