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

Significantly improve fetch speed of bottles #15579

Merged
merged 1 commit into from Jun 22, 2023

Conversation

Bo98
Copy link
Member

@Bo98 Bo98 commented Jun 22, 2023

Now it's time to look at fetching.

On my machine, using hello as a test, this PR:

  • Improves brew fetch speeds by ~4 seeconds if the bottle and manifest were not previously cached.
  • Improves brew fetch speeds by ~2 seconds (to now be effectively 0 time beyond brew startup, formula loading etc) if the bottle and manifest have been previously cached.

The improvements in this PR are largely specific to the GitHub Packages download strategy, where I have added some shortcutting due to assumptions we can make based on how GitHub Packages downloads work. Specifically, I've removed all header checks which means that:

  • The filename in Content-Disposition is no longer checked
    • We override the filename choice and the remote filename was blank anyway in GHCR
  • Last-Modified is no longer checked for a stale cache
    • This was never set for maninfest anyway, so for that we already have other checks to redownload it if necessary
    • For the bottle itself, those are keyed by sha256 in the URL. So it is only necessary to check if a SHA256 clash occurred, which we can probably ignore.
  • Content-Length is no longer checked
    • I believe this was only used in casks?
  • Redirections are no longer checked

No redirection checks does have some security effects worth noting (remember these only apply to bottle and manifest downloads - source downloads are unchanged):

  • Redirects are no longer reported to the user
    • We have always assumed the root URL is trusted and third-party taps and mirrors should use HTTPS to make sure the server is trusted.
  • HOMEBREW_NO_INSECURE_REDIRECT no longer has any effect on bottles
  • Older curls affected by CVE-2018-1000007 (macOS <10.15, Ubuntu <18.04) will now pass the Authorization header to redirects for bottle and manifest downloads. This is not a concern for homebrew-core because the auth header is bogus and not a secret, but may be a concern for private package repositories. It's not entirely clear whose reponsibility this is. I could maybe add some workarounds for that if desired.

This PR also has the following changes that impact all downloads (bottle and source):

  • Fixed partial download support detection not working correctly, causing download resuming to never happen.
  • Skipped partial download header checks if there is no cached file. This improves fetch speeds by a couple seconds for me.

Combining this and the install chages, give brew install hello a try with and without cached downloads.

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.

Wonderful, fantastic work again @Bo98, you're on a roll!

@MikeMcQuaid MikeMcQuaid merged commit 13d99db into Homebrew:master Jun 22, 2023
22 checks passed
@Bo98 Bo98 deleted the faster-fetch branch June 22, 2023 14:22
@github-actions github-actions bot added the outdated PR was locked due to age label Jul 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 23, 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

2 participants