search

php-http / message

HTTP Message related tools
http://php-http.org
MIT License
1.3k stars 42 forks source link

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

Open stof opened 6 years ago

stof commented 6 years ago

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)

Nyholm commented 6 years ago

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.

stof commented 6 years ago

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

Nyholm commented 6 years ago

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 notifications@github.com 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.

stof commented 6 years ago

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)