message icon indicating copy to clipboard operation
message copied to clipboard

MultipartStreamBuilder uses too much memory when building the stream with a big body

Open stof opened this issue 7 years ago • 4 comments

Currently, MultipartStreamBuilder builds the whole content of the stream as a string in memory, even when the original data was built using a Stream wrapping a resource to avoid loading the whole file in memory at this point.

As building a multipart stream only involves appending the different strings with some delimiters, it would be much better to implementing this in a streaming way.

GuzzleHttp\Psr7\AppendStream might help here (which is what GuzzleHttp\Psr7\MultipartStream uses internally)

stof avatar Apr 05 '18 12:04 stof

Yeah. I know. Hm. If I remember correctly, I had a hard time achieving this working with the PSR7 interfaces only. But it should be revisited.

Nyholm avatar Apr 05 '18 16:04 Nyholm

Well, AFAIK, AppendSteam is a pure-PSR-7 stream wrapping other ones. And MultipartStream builds on top of it.

In the meantime, I might investigate using GuzzleHttp\Psr7\MultipartStream instead (as I'm using guzzlehttp/psr7 as PSR-7 implementation anyway

stof avatar Apr 05 '18 16:04 stof

Yeah. If you are developing an application you should consider a specific implementation.

AppendStream is using guzzle specific stuff. That is why I didn’t port it to a new package but created MultipartStreamBuilder.

//Tobias Nyholm

On 5 Apr 2018, at 18:32, Christophe Coevoet [email protected] wrote:

Well, AFAIK, AppendSteam is a pure-PSR-7 stream wrapping other ones. And MultipartStream builds on top of it.

In the meantime, I might investigate using GuzzleHttp\Psr7\MultipartStream instead (as I'm using guzzlehttp/psr7 as PSR-7 implementation anyway

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

Nyholm avatar Apr 05 '18 17:04 Nyholm

quick idea: detect whether the GuzzleHttp\Psr7\MultipartStream class exist, and use it in the implementation of MultipartStreamBuilder instead of building a stream in memory (the stream built in memory can still be used as fallback). This would allow giving a better experience (i.e. not OOM errors) to people using guzzlehttp/psr7.

This would only involve moving the header preparation to the build time (or least storing the name, to be able to pass it to guzzle, which will ignore it because the content-disposition header would be built already)

stof avatar Apr 10 '18 16:04 stof