Skip to content

proposal: compress: add Compress and Decompress convenience functions #16504

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

Closed
dsnet opened this issue Jul 26, 2016 · 7 comments
Closed

proposal: compress: add Compress and Decompress convenience functions #16504

dsnet opened this issue Jul 26, 2016 · 7 comments

Comments

@dsnet
Copy link
Member

dsnet commented Jul 26, 2016

Filing on behalf of an internal user, who requests that we add convenience wrappers of Compress and Decompress:

func Decompress(b []byte) ([]byte, error) {
    rd, err := NewReader(bytes.NewReader(b))
    if err != nil {
        return nil, err
    }
    d, err := ioutil.ReadAll(rd)
    if cerr := rd.Close(); err == nil {
        err = cerr
    }
    return d, err
}

What are people's thoughts?

@dsnet dsnet added the Proposal label Jul 26, 2016
@nhooyr
Copy link
Contributor

nhooyr commented Jul 26, 2016

If we do this, we should use a sync.Pool for the readers.

@bradfitz
Copy link
Contributor

Maybe things have changed, but I would expect pushback for sync.Pool use.

@odeke-em
Copy link
Member

Just like net/http has:

  • Get.
  • Post.
  • Head.
  • PostForm

etc
that underlyingly use the default client's methods perhaps each compress package could have its own Compress and Decompress convenience functions, instead of a default of zlib?

  • compress/zlib.[Compress, Decompress]
  • compress/lzw.[Compress, Decompress]
  • compress/gzip.[Compress, Decompress]
  • compress/bzip2.[Compress, Decompress]

Apologies if am preaching to the choir but it wasn't straight forward for me from your example if the convenience functions would be in the top level compress or in each package.

@dsnet
Copy link
Member Author

dsnet commented Jul 27, 2016

@odeke-em, the suggestion in the original post is for exactly what you listed out. Thanks for the asking to clarify.

@dsnet dsnet added this to the Unplanned milestone Jul 28, 2016
@ulikunitz
Copy link
Contributor

If convenience functions are required for compression packages I suggest the alternatives:

func Compress(w io.Writer, data []byte) error
func Decompress(r io.Reader) (data []byte, err error)

These functions would have more use cases and are very easy to use with a bytes.Buffer or a bytes.Reader.

var buf bytes.Buffer
err := Compress(&buf, data)
data, err := Decompress(bytes.NewReader(compressedData))

BTW Decompression doesn't need the Close call. It doesn't do anything useful. Depending on the compression package Close for a reader either sets the reader to closed or returns the last returned error if its not io.EOF. IMHO returning a io.ReadCloser for a decompressor is a design error. The bzip2 package does the right thing and returns only an io.Reader.

@bradfitz
Copy link
Contributor

Yeah, I don't think helpers which slurp everything to memory are helpful. You're compressing it in the first place because it's large, so pulling it all into memory when it's almost certainly large is not a good idea.

I think the existing Reader- & Writer-based interfaces are the right answer.

Maybe we just need more documentation?

@bradfitz
Copy link
Contributor

Sorry, going to close this one. We already have @ulikunitz's func Decompress in the form of ioutil.ReadAll anyway.

@golang golang locked and limited conversation to collaborators Sep 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants