Open codercms opened 3 years ago
@codercms are you always experiencing that low performance or it depends on the text length? Does anything change if /maxmemory:NN
gets added to php://temp
? The filesystem should only be used when the text is big (default limit is 2Mb).
my suggestion is to implement pure RAM string stream.
Overall agree, php://memory
could suffice here, but we also are not sure if the provided string|StreamInterface
will fit in memory, hence why a more conservative php://temp
was used here.
I suggest sending a patch, and then we discuss the implications there: it is also acceptable to tell end-users that TextResponse
is not intended for large payloads (which is in fact the original design anyway)
@lcobucci there is two cases:
Patched TextReponse:
$size = 10 * 1024 * 1024;
$body = new Stream("php://temp/maxmemory:{$size}", 'wb+');
It is still slower than approach without PHP native stream allocation. 320 RPS without native PHP stream 260 RPS with native PHP stream
@Ocramius I guess this patch will not affect end users, because php native stream is only allocated when string was passed to the constructor - this means that the memory for response is already allocated.
Looking back at this, I think that for text, we can use php://memory
, and let consumers with large text/plain
payloads implement their own streamed response perhaps?
Unsure if this should be considered a BC break.
Similar for html
, which is the more common response object.
@Ocramius I think there is one more approach (in two different ways):
I didn't really understand the difference between those suggested approaches :thinking:
@Ocramius actually its only one approach which can be implemented in two manners
But basically, if I understand it correctly, avoiding usage of a stream at all, and having a string
variable instead (since html
/text
responses are generally well within the memory limits)
@Ocramius yes it is. The performance problem occurs when the Response object allocates stream for string
(I mean PHP stream), but actually there is no need to allocate memory one more time.
Feature Request
Summary
Currently
TextResponse
always allocating new buffer for string output - https://github.com/laminas/laminas-diactoros/blob/2.6.x/src/Response/TextResponse.php#L75 This approach working really slow, my suggestion is to implement pure RAM string stream. Here is what I mean - https://github.com/makise-co/framework/blob/master/src/Http/FakeStream.php (this approach is ~30% more performant)