-
-
Notifications
You must be signed in to change notification settings - Fork 76
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
Always store Response in memory #227
Conversation
Can you back both claims with some data? I never heard about a system without a tmp directory so claim b) looks non-existant to me, and claim a) would need a scenario where this matters, and I've yet to see one. Also, this ignores the memory consumption this change would incur.
|
The memory consumption should be ok as the code is reached from a string already present in memory. If the source data were not a string (Stream or resource), the code is not reached.
This is how I discovered this problem, we have been observing high SSD usage, as our API responses are often above 2MB |
Ah, makes sense now thanks for the insights. |
The data are duplicated. |
I think it's possible to create a stream wrapper to prevent the duplication. I would take https://www.php.net/manual/en/stream.streamwrapper.example-1.php as a start and instead of relying on globals, I would initialize the buffer via a custom stream context. But I'm not saying this to block this PR. This could be contributed later. |
Idea: add a stream context option to the php://memory stream wrapper to initialize the buffer on fopen and prevent the duplication natively. Could you open an issue on php-src so that the idea can be considered? |
I do not think the data are internally stored in zval. I would say the custom stream wrapper can be a solution to the duplication, but I am not fully sure if it is worth it, as normally the string is build, released, and thus no deduplication is needed. |
This is quite a breaking change, and will break things for people using low-memory environments, who need to move things to disk. |
Yes, but as this change affects only input already stored in memory/string, the memory consumption should not be a major BC-break. |
Fair. |
I propose #230 instead. |
closing in favor of #230 |
Response buffer should always stay in memory for two reasons:
man https://www.php.net/manual/en/wrappers.php.php