Skip to content
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

cmd/audit: improve performance of versioned formula names #16010

Conversation

apainintheneck
Copy link
Contributor

@apainintheneck apainintheneck commented Sep 16, 2023

  • 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?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

There is a check for other versioned formula with the same name in the file audit. This just memoizes all of the versioned formulae in a tap during the first call and then uses that much shorter list every time it checks for things.

Command:

brew prof --  audit --skip-style --except=version,installed --tap=homebrew/core

Before:
Screen Shot 2023-09-16 at 9 07 06 AM

After:
Screen Shot 2023-09-16 at 9 12 37 AM

It results in a 5-10 second speed up locally for me.

Update: It looks like we're down to ~35 seconds in this repo's CI after this change and @Bo98's change in #16008.

Screen Shot 2023-09-16 at 9 28 34 AM

There is a check for other versioned formula with the same name
in the file audit. This just memoizes all of the versioned
formulae in a tap during the first call and then uses that much
shorter list everytime it checks for things.
@Bo98
Copy link
Member

Bo98 commented Sep 16, 2023

Yeah I had identified the same: https://github.com/Homebrew/brew/pull/16007/files#diff-61f7b22029742d3ce5a09055c06c73368cc9a06b965af5c172553731eb0855b6R650

Looks like your version is a little more fleshed out though.

@apainintheneck apainintheneck marked this pull request as ready for review September 16, 2023 16:40
@apainintheneck
Copy link
Contributor Author

@Bo98 I'd overlooked that when reviewing your PR. My bad.

@apainintheneck
Copy link
Contributor Author

It looks like this also speeds up formula API generation as well.

Before: (from #16008)

Screen Shot 2023-09-16 at 9 56 37 AM

After:

Screen Shot 2023-09-16 at 9 57 20 AM

@apainintheneck apainintheneck merged commit aa2a77b into Homebrew:master Sep 17, 2023
24 checks passed
@MikeMcQuaid
Copy link
Member

Great work as usual @apainintheneck!

@zouchengli
Copy link

lima@lima-ubuntu-lts:~$ ruby -v
ruby 2.6.10p210 (2022-04-12 revision 67958) [aarch64-linux]

lima@lima-ubuntu-lts:~$ echo $PATH
/home/lima.linux/.rbenv/shims:/home/lima.linux/.rbenv/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/usr/sbin:/sbin:/usr/sbin:/sbin:/usr/sbin:/sbin:/home/lima.linux/.rbenv/versions/2.6.10/bin/

lima@lima-ubuntu-lts:~$ ./install.sh
==> Checking for sudo access (which may request your password)...
==> This script will install:
/home/linuxbrew/.linuxbrew/bin/brew
/home/linuxbrew/.linuxbrew/share/doc/homebrew
/home/linuxbrew/.linuxbrew/share/man/man1/brew.1
/home/linuxbrew/.linuxbrew/share/zsh/site-functions/_brew
/home/linuxbrew/.linuxbrew/etc/bash_completion.d/brew
/home/linuxbrew/.linuxbrew/Homebrew

Press RETURN/ENTER to continue or any other key to abort:
==> /usr/bin/sudo /bin/chown -R lima:lima /home/linuxbrew/.linuxbrew/Homebrew
==> Downloading and installing Homebrew...
HEAD is now at aa2a77b Merge pull request #16010 from apainintheneck/improve-versioned-formula-names-performance
Error: No Homebrew ruby 2.6.10_1 available for aarch64 processors!
Error: Failed to install Homebrew Portable Ruby and cannot find another Ruby 2.6.10!
If there's no Homebrew Portable Ruby available for your processor:

  • install Ruby 2.6.10 with your system package manager (or rbenv/ruby-build)
  • make it first in your PATH
  • try again

Failed during: /home/linuxbrew/.linuxbrew/bin/brew update --force --quiet

@apainintheneck
Copy link
Contributor Author

@zouchengli It's not obvious from the stack trace that this problem is related to this PR. Could you either make an issue in the install repo or open a discussion so that we can better troubleshoot this?

@zouchengli
Copy link

@zouchengli It's not obvious from the stack trace that this problem is related to this PR. Could you either make an issue in the install repo or open a discussion so that we can better troubleshoot this?

OK Please refer to Try install homebrew on arm64 ubuntu

@github-actions github-actions bot added the outdated PR was locked due to age label Oct 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants