perf(response): Flush requests early to clients#57991
perf(response): Flush requests early to clients#57991nickvergessen wants to merge 1 commit intomasterfrom
Conversation
If not, the response will wait for async actions, e.g. HTTP requests from IClientService, to be finished before returning. This is not desired in many cases with e.g. with notifications. Signed-off-by: Joas Schilling <coding@schilljs.com>
nickvergessen
added
3. to review
nickvergessen
requested review from
ArtificialOwl,
icewind1991,
leftybournes and
salmart-dev
and removed request for
a team
There was a problem hiding this comment.
Small comments, but I like the idea :)
There is also fastcgi_finish_request for FPM, it may worth a try as it also kills the connection
| } | ||
|
|
||
| if ($response->getFlushEarly()) { | ||
| ob_flush(); |
There was a problem hiding this comment.
| ob_flush(); | |
| while (ob_get_level() > 0) { | |
| ob_end_flush(); | |
| } |
I would go with this snippet to ensure all is flushed
| public function setFlushEarly(bool $flushEarly): void { | ||
| $this->flushEarly = $flushEarly; | ||
| } |
There was a problem hiding this comment.
Since the default is to flush early, maybe this is more explicit:
| public function setFlushEarly(bool $flushEarly): void { | |
| $this->flushEarly = $flushEarly; | |
| } | |
| public function dontFlushEarly(): void { | |
| $this->flushEarly = false; | |
| } |
There was a problem hiding this comment.
Since the default is to flush early
That's TBD. It could be problematic in some cases, but in theory it could break things unintentionally, or at least be a behaviour change
|
From PHP comments a small reminder:
We should ensure session was closed before. |
|
Good remark about session. I think one of the best scenario would be:
I didn't check what is possible. It may be possible to reduce session access even more |
2 participants
If not, the response will wait for async actions, e.g. HTTP requests from IClientService, to be finished before returning. This is not desired in many cases with e.g. with notifications.
Checklist
3. to review, feature component)stable32)