-
-
Notifications
You must be signed in to change notification settings - Fork 10.6k
formula: default to cache for Dependency#expand usage
#20827
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Wasn't sure if there was any reasonable use case of Alternatively, could add |
fb17e13 to
ebb2b67
Compare
Dependency#expand usage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine! When is use_cache: false used?
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
|
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L
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: