FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Bundle installation performance improvements by henrymercer · Pull Request #2564 · github/codeql-action · GitHub

/ codeql-action Public

Bundle installation performance improvements - #2564

Merged
henrymercer merged 4 commits into
mainfrom
henrymercer/only-use-stdin-for-streaming
Oct 23, 2024
ConversationCommits (4)ChecksFiles changed
Merged

Bundle installation performance improvements#2564
henrymercer merged 4 commits into
mainfrom
henrymercer/only-use-stdin-for-streaming

Conversation

henrymercer commented Oct 22, 2024

Copy link
Copy Markdown
Contributor

Make some performance improvements to bundle installation:

  • Only stream the download and extraction of bundles on Linux.
  • Only pass the archive to tar via stdin when we're streaming the download and extraction.
  • Bump the high water mark when streaming the download and extraction of the bundle to 16 MB to hopefully reduce context switching.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.
  • Confirm the changelog has been updated if necessary.

henrymercer requested a review from a team as a code owner October 22, 2024 21:07
aeisenberg approved these changes Oct 22, 2024
View reviewed changes

aeisenberg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Seems reasonable. Do you have an idea how much time is being saved?

henrymercer commented Oct 23, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Good question:

  • Passing the archive via stdin seems to slow things down on macOS and Windows — on macOS from ~23s to ~28s, on Windows it is difficult to get a precise measurement since we only use zstd when explicitly specified.
  • Streaming on macOS (~20s) is faster than passing the archive via stdin (~28s), but has bad p95 performance (several minutes), so it is more reliable to download in full and pass the file to tar (~23s).
  • The high water mark I didn't observe anything over a small number of testing runs, but will see if there's a difference at larger scale.

henrymercer merged commit b97ec3a into main Oct 23, 2024
henrymercer deleted the henrymercer/only-use-stdin-for-streaming branch October 23, 2024 09:54
This was referenced Nov 8, 2024
Merged
Merged
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

aeisenberg aeisenberg approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL