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: only flush formulary cache when needed #15987

Conversation

apainintheneck
Copy link
Contributor

@apainintheneck apainintheneck commented Sep 10, 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?

The cache here needs to be cleared when we are auditing multiple os/arch combinations but not when we are running the audit only for the current os/arch combination. This gives a 2x speed boost to brew audit --skip-style --except-version --tap=homebrew/core locally.

In theory we could clear the cache less often by running all audits for one os/arch combination at a time when auditing multiple os/arch combinations. This would make the output a bit harder to follow and we don't seem to run those checks on CI so I'm skipping that for now.

Note: This is on an old mac so we'll see how much faster it looks in CI.
Edit: A comparable PR took 1m 25s and this one took 53s for the core audit step.

/u/l/Homebrew (master|✚1) $ hyperfine -M 3 'brew audit --skip-style --except=version --tap=homebrew/core' 'brew audit --skip-style --except=version --os=catalina --tap=homebrew/core'
Benchmark 1: brew audit --skip-style --except=version --tap=homebrew/core
  Time (mean ± σ):     148.105 s ±  0.849 s    [User: 106.567 s, System: 31.634 s]
  Range (min … max):   147.130 s … 148.682 s    3 runs

Benchmark 2: brew audit --skip-style --except=version --os=catalina --tap=homebrew/core
  Time (mean ± σ):     309.744 s ±  1.651 s    [User: 252.120 s, System: 45.346 s]
  Range (min … max):   307.917 s … 311.130 s    3 runs

Summary
  brew audit --skip-style --except=version --tap=homebrew/core ran
    2.09 ± 0.02 times faster than brew audit --skip-style --except=version --os=catalina --tap=homebrew/core

Before

/u/l/Homebrew (master|✔) [SIGINT]$ brew audit --skip-style --except=version -D --tap=homebrew/core
audit_style:                        0.0101 sec
audit_bitbucket_repository:         0.0104 sec
audit_gitlab_repository:            0.0112 sec
audit_versioned_keg_only:           0.0131 sec
audit_github_repository:            0.0132 sec
audit_bottle_spec:                  0.0135 sec
audit_revision_and_version_scheme:  0.0146 sec
audit_reverse_migration:            0.0181 sec
audit_glibc:                        0.0205 sec
audit_hashicorp_formulae:           0.0206 sec
audit_postgresql:                   0.0208 sec
audit_elasticsearch_kibana:         0.0234 sec
audit_formula_name:                 0.0295 sec
audit_gcc_dependency:               0.0301 sec
audit_homepage:                     0.0376 sec
audit_gitlab_repository_archived:   0.0390 sec
audit_keg_only_reason:              0.0428 sec
audit_github_repository_archived:   0.0585 sec
audit_license:                      0.7185 sec
audit_prefix_has_contents:          0.7324 sec
audit_conflicts:                    1.2574 sec
audit_synced_versions_formulae:     2.1366 sec
audit_text:                         2.3681 sec
audit_specs:                        6.3412 sec
audit_file:                         10.6020 sec
audit_installed:                    98.6924 sec
audit_deps:                         152.9530 sec

After

/u/l/Homebrew (master|✚1) $ brew audit --skip-style --except=version -D --tap=homebrew/core
audit_bottle_spec:                  0.0085 sec
audit_style:                        0.0089 sec
audit_gitlab_repository:            0.0089 sec
audit_bitbucket_repository:         0.0092 sec
audit_github_repository:            0.0104 sec
audit_reverse_migration:            0.0110 sec
audit_versioned_keg_only:           0.0111 sec
audit_revision_and_version_scheme:  0.0120 sec
audit_hashicorp_formulae:           0.0165 sec
audit_glibc:                        0.0167 sec
audit_postgresql:                   0.0172 sec
audit_elasticsearch_kibana:         0.0190 sec
audit_gcc_dependency:               0.0224 sec
audit_formula_name:                 0.0254 sec
audit_keg_only_reason:              0.0301 sec
audit_gitlab_repository_archived:   0.0327 sec
audit_homepage:                     0.0331 sec
audit_github_repository_archived:   0.0458 sec
audit_conflicts:                    0.0682 sec
audit_license:                      0.5847 sec
audit_prefix_has_contents:          0.7994 sec
audit_synced_versions_formulae:     1.2405 sec
audit_text:                         2.2840 sec
audit_specs:                        5.7359 sec
audit_file:                         6.1694 sec
audit_deps:                         24.2161 sec
audit_installed:                    76.9011 sec

The cache here needs to be cleared when we are auditing multiple
os/arch combinations but not when we are running the audit only
for the current os/arch combination. This gives a 2x speed boost
to `brew audit --skip-style --except-version --tap=homebrew/core` locally.

In theory we could clear the cache less often by running all audits for
one os/arch combination at a time when auditing multiple os/arch combinations.
This would make the output a bit harder to follow and we don't seem to
run those checks on CI so I'm skipping that for now.
@apainintheneck apainintheneck marked this pull request as ready for review September 10, 2023 02:16
@apainintheneck
Copy link
Contributor Author

apainintheneck commented Sep 10, 2023

It's probably not even useful to enable the Formulary cache in the first place if we're going to be clearing it over and over again. I'll see if there's any noticeable performance boost there.

Edit: Never mind that made things worse.

Copy link
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.

@apainintheneck Great work here, very nice!

@MikeMcQuaid MikeMcQuaid merged commit 838cb3b into Homebrew:master Sep 11, 2023
24 checks passed
@github-actions github-actions bot added the outdated PR was locked due to age label Oct 12, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 12, 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.

2 participants