missing content-length and ioutil.ReadAll usage

[axyz]

@aryszka @rgritti
after profiling and load testing a skrop based app I found a possible source of considerable performance penalties:

skrop/filters/imagefilter.go

Line 222 in 59b76fe

buf, err := ioutil.ReadAll(rsp.Body)

when the image get read from the body and saved on the stateBag ioutil.ReadAll is used. This will start from a very small buffer and will keep duplicating the size in order to fit the image. Considering the source images are usually pretty big in terms of size, even with an exponential growth the buffer will need to be doubled a lot of times. To do that internally the Bytes.MakeSlice function will be called. This function seems not to be very efficient and under an even very moderate load will rapidly occupy the biggest part of the flame graph (usually around 70/80%). Even when the initial slice size is similar to the input and the buffer is doubled a single time, the performance impact is very high.

To avoid the usage of ReadAll something like this can be used:

buf := bytes.NewBuffer(make([]byte, 0, rsp.ContentLength)) // eventually some extra bytes could be added to make sure no makeSlice will happen
_, err := buf.ReadFrom(rsp.Body)

// use buf.Bytes() to pass around the read bytes

of course if content-length is not set in the source there is not much we can do, but one idea may be to have an option to have a fallback initial buffer size that can be customized accordingly to the average size of the inputs.

Playing around with this approach I also discovered another strange behavior:

skrop/filters/imagefilter.go

Line 185 in 59b76fe

rsp.Body = ioutil.NopCloser(bytes.NewReader(buf))

during the FinalizeResponse the body is set to a buffer reader, I was expecting that the content-length would be set automatically accordingly to the size of the buffer, but it is actually not set, this means that having a loopback filter that will have to read from the body will lead to the same fallback case where we cannot rely on content-length in order to initialize our buffer. I tried to manually set rsp.ConentLength to the length of the buffer, but I'm not sure that is the most idiomatic way to solve the issue and it also look like the buffer size is not enough to avoid a buffer doubling and some extra bytes need to be added (tried with 4k and it never called makeSlice)