lz4 decompression causes a lot of allocations due to ioutil.ReadAll

[kalbhor]

While using a consumer group where messages were lz4 compressed I noticed most of the allocations were due to ioutil.ReadAll. Should we consider using io.Copy (fixed buffer length) instead of ioutil.ReadAll (dynamically resized array)? I can provide benchmark results if needed

[twmb]

What does code look like that avoids ReadAll? io.Copy requires a destination to read into, and I don't think we can know the size of what the blob will decompress into ahead of time.

[kalbhor]

Since we don't know the final buffer size, we can't pre-allocate. But it looks like bytes.Buffer has a more optimal grow buffer method for most cases. Since ReadAll depends on append's grow strategy, it is generally discouraged for reading large data in-memory. There are discussions around this : golang/go#50774

I used such an io.Copy example for benchmarks:

func decompressCopy(d []byte) ([]byte, error) {
	dc := dcPool.Get().(*lz4.Reader)
	defer dcPool.Put(dc)
	dc.Reset(bytes.NewReader(d))

	out := new(bytes.Buffer)
	if _, err := io.Copy(out, dc); err != nil {
		return nil, err
	}

	return out.Bytes(), nil
}

[twmb]

Sure, makes sense 👍

[kalbhor]

I found that gzip decompress also uses ioutil.ReadAll and could in most cases benefit from a bytes.Buffer. If you like I can open a pull request with these changes

[twmb]

Sure, also good. +1 to PR

[twmb]

Let me know if you can get a PR open in the next day or two -- I have two other PRs ready that can go into a patch release and this can fit as well.

[kalbhor]

Replaced ReadAll with bytes buffer for gzip and lz4 decompression