Use multi-core optimized pgzip package in tarball compressor #98
Conversation
fileutil/tarball_compressor.go
Outdated
| return bosherr.WrapError(err, "Decompressing file contents") | ||
| } | ||
| default: | ||
| return fmt.Errorf("uknown type: %v in %s for tar: %s", |
fileutil/tarball_compressor.go
Outdated
| "path/filepath" | ||
|
|
||
| "archive/tar" | ||
| gzip "github.com/klauspost/pgzip" |
There was a problem hiding this comment.
Maybe drop this import alias? It only hides information.
fileutil/tarball_compressor.go
Outdated
| } | ||
|
|
||
| if fi.Mode().IsRegular() { | ||
| data, err := os.Open(f) |
There was a problem hiding this comment.
should this opened file be closed or is handled by the close of the tarball defer tarball.Close()?
fileutil/tarball_compressor.go
Outdated
| tarball, err := c.fs.OpenFile(tarballPath, os.O_RDONLY, 0) | ||
| if err != nil { | ||
| return bosherr.WrapError(err, "Opening tarball") | ||
| } | ||
|
|
||
| if options.PathInArchive != "" { | ||
| args = append(args, options.PathInArchive) | ||
| } | ||
| _, _, _, err := c.cmdRunner.RunCommand("tar", args...) | ||
| zr, err := gzip.NewReader(tarball) |
There was a problem hiding this comment.
Should we add defer tarball.Close() and defer zr.Close()? The CompressFilesInDir function is closing the tarball stream e.g.. Or is the close handled differently here because I couldn't follow where the close is happening?
The maintainer is still active on issues (the most recent one is from February): klauspost/pgzip#56 (comment) So to me that suggests the project is feature-complete. It also has a healthy number of github stars. |
- don't use import alias - close all readers / writers - fix typo - also use bosh filesystem interface when reading file contents
|
Waiting for #99 |
Some early benchmarks (when using these changes in the bosh-cli):
I did make this a breaking change to the interface because we no longer need the CmdRunner in the TarballCompressor.