experimental/air: parallel gzip for the plain_tar snapshot packer - #6571
Merged
Conversation
This was referenced Sep 8, 2026
ben-hansen-db
marked this pull request as ready for review
September 8, 2026 17:42
vinchenzo-db
approved these changes
Sep 8, 2026
| @@ -9,6 +9,8 @@ import ( | |||
| "os/exec" | |||
| "path/filepath" | |||
| "strings" | |||
|
|
|||
Contributor
There was a problem hiding this comment.
is Go semantics/lint to have this empty space? if not then rm
Contributor
Author
There was a problem hiding this comment.
need to keep it according to claude:
that blank line is the standard import grouping (stdlib vs third-party) that goimports/gofumpt enforce, and this repo runs both as formatters. Without it, gofmt sorts github.com/klauspost/pgzip alphabetically into the stdlib block (between fmt and os) and goimports re-adds the separator, so ./task fmt would put it right back.
Comment on lines
+69
to
+71
| // level 9 buys almost nothing beyond that for ~2x the time. Compressing outside tar | ||
| // also passes no archive path to tar, sidestepping the Windows colon-in-path issue a | ||
| // `-f <path>` argument otherwise hits (tar reads the `C:` in `C:\out\x` as a host). |
Contributor
There was a problem hiding this comment.
Either dumb down this part or rm imo
createPlainTarball shelled out to `tar -czf`, whose gzip is single-threaded and dominates packaging time for a large code_source tree. Pipe `tar -cf -` through klauspost/pgzip instead, spreading compression across cores. Measured ~4x on universe/research (2456 ms -> 609 ms) and ~18x on the 470 MiB gzip step alone; the output is an ordinary gzip stream. Level is BestSpeed since the uploaded size does not matter for this workflow, only latency. Compressing outside tar also passes no archive path to tar, which removes the Windows colon-in-path workaround (bare `-f` basename + -C) the -czf form needed. Co-authored-by: Isaac <no-reply@databricks.com>
Now that gzip is parallel the compression level is nearly free, so trade a little CPU for a smaller upload -- the plain_tar archive is re-uploaded on every run. DefaultCompression matches the old `tar -czf` size at ~18x the speed (research: 716 ms / 24 MB, vs BestSpeed 609 ms / 27 MB). Level 9 buys ~1% fewer bytes for ~2x the time, so 6 is the knee. Co-authored-by: Isaac <no-reply@databricks.com>
Vincent found the pack-step comment too dense. Cut it from the compression-level essay (that rationale lives in the PR description and commit message) down to the two non-obvious whys: parallel gzip vs tar's single-threaded -z, and compressing outside tar to avoid the Windows colon-in-path issue. Co-authored-by: Isaac <no-reply@databricks.com>
ben-hansen-db
force-pushed
the
air-plain-tar-pgzip
branch
from
September 8, 2026 21:52
b732496 to
8155ccf
Compare
Collaborator
Integration test reportCommit: 660b22e
Top 6 slowest tests (at least 2 minutes):
|
pietern
approved these changes
Sep 10, 2026
… out to tar Per review, replace the `tar -cf -` subprocess in createPlainTarball with a Go-native tar writer. Add libs/tarpack, a small helper that writes a tar stream to a caller-supplied io.Writer, so compression stays the caller's choice: air run wraps it in klauspost/pgzip, keeping the parallel-gzip win while dropping the external tar dependency and its Windows colon-in-path workaround. Symlinks, file modes, and mtimes are preserved, matching what tar stored. Enumeration (git ls-files) and the git-ref path (git archive) are unchanged. The helper lives under libs/ so other packers can adopt it, but only air run is wired to it here. Co-authored-by: Isaac <no-reply@databricks.com>
Collaborator
Integration test reportCommit: 6ec1ee1
492 interesting tests: 486 FAIL, 5 KNOWN, 1 SKIP
Top 50 slowest tests (at least 2 minutes):
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
air runplain_tar snapshot path (dirty working tree / no git ref) packages the code_source by shelling out totar -czf. tar's built-in gzip is single-threaded, so for a large tree it dominates packaging latency.This pipes
tar -cf -(uncompressed) through klauspost/pgzip (MIT), a parallel drop-in for gzip that spreads compression across cores and still emits an ordinary gzip stream.tar -czf, not BestSpeed. The archive is re-uploaded on every run, so its size matters. So this is a pure latency win with no upload-size regression.-f <path>form required.Results (local packaging: git walk + tar + gzip; warm page cache)
tar -czf, serial gz6)tar -cf -| pgzip, gz6)Same compression level, so the tarball is the same size as before (~24 MB for large folder). Level chosen from the curve on a 476 MiB tar: L1→L6 costs +150 ms for ~17% fewer bytes (97→80 MB); L6→L9 doubles time for ~1%, so 6 is the knee.
Validation
gzip -tclean, entry count matches the file list, extracts correctly.snapshot_package_test.gounit tests pass; theinternal/buildlicense test passes for the newpgzipdep (// MITin go.mod + NOTICE entry).