experimental/air: parallel gzip for the plain_tar snapshot packer - #6571
experimental/air: parallel gzip for the plain_tar snapshot packer#6571ben-hansen-db wants to merge 3 commits into
Conversation
Approval status: pending
|
| @@ -9,6 +9,8 @@ import ( | |||
| "os/exec" | |||
| "path/filepath" | |||
| "strings" | |||
|
|
|||
There was a problem hiding this comment.
is Go semantics/lint to have this empty space? if not then rm
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.
| // 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). |
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>
b732496 to
8155ccf
Compare
Integration test reportCommit: 8155ccf
Top 7 slowest tests (at least 2 minutes):
|
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 research). 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).Follow-up
A separate PR (#6572) adds a warm snapshot cache on top of this, which helps most for large monorepos and cold page cache.
This pull request and its description were written by Isaac.