Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

SapiStreamEmitter::emit() - high memory usage - memory exhaust #223

Closed
fcabralpacheco opened this issue Jan 14, 2017 · 1 comment
Closed
Assignees
Labels
Milestone

Comments

@fcabralpacheco
Copy link
Contributor

I'm working on a project where the application acts as proxy/cache for internal network requests.

When SapiStreamEmitter::emit() is called, it ends up generating a high memory usage, or reaches the limits imposed by the PHP configuration.

This only occurs when the stream is not seekable, and SapiStreamEmitter::emit() uses the stream::toString() or stream::getContents() to get the full contents from stream and store it in memory (eg: echo $body;).

At first, I looked for other related issues, and by the comments I accepted that the behavior of the emitter was the expected, and ended up creating a custom emitter for application.

But over time I started to think that it was strange by default to use a stream and not make use of its capabilities to reduce memory usage.

I understand that the stream::toString() and stream::getContents() performance are superior to a sequence of stream::read(), but I think in a more general way when using streams, you can use the stream::read() when the stream is readable, and not only when it is seekable.

I implemented a possible fix:

fcabralpacheco@c90ca44

Do you think this would be a good solution?

@fcabralpacheco fcabralpacheco changed the title SapiStreamEmitter::emit - high memory usage - memory exhaust SapiStreamEmitter::emit() - high memory usage - memory exhaust Jan 14, 2017
@Ocramius
Copy link
Member

This only occurs when the stream is not seekable, and SapiStreamEmitter::emit() uses the stream::toString() or stream::getContents() to get the full contents from stream and store it in memory (eg: echo $body;).

stream::toString() and stream::getContents() will obviously cause a lot of memory usage.

The patch you provided looks interesting for readable streams: can you send it as a pull request for review?

@Ocramius Ocramius added the bug label Jan 14, 2017
Ocramius added a commit that referenced this issue Jan 17, 2017
Ocramius added a commit that referenced this issue Jan 17, 2017
Ocramius added a commit that referenced this issue Jan 17, 2017
Ocramius added a commit that referenced this issue Jan 17, 2017
Ocramius added a commit that referenced this issue Jan 17, 2017
Ocramius added a commit that referenced this issue Jan 17, 2017
Ocramius added a commit that referenced this issue Jan 17, 2017
@Ocramius Ocramius added this to the 1.3.9 milestone Jan 17, 2017
@Ocramius Ocramius self-assigned this Jan 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants