Skip to content
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

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

Open
stof opened this issue Apr 5, 2018 · 4 comments

Comments

@stof
Copy link

stof commented Apr 5, 2018

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
Copy link
Member

Nyholm commented Apr 5, 2018

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
Copy link
Author

stof commented Apr 5, 2018

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
Copy link
Member

Nyholm commented Apr 5, 2018 via email

@stof
Copy link
Author

stof commented Apr 10, 2018

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants