Improve memory efficiency #18932
Improve memory efficiency #18932
Conversation
This avoids concatenating `$message` and `PHP_EOL` (if necessary) as a new value, greatly improving memory efficiency for large `$message`s.
|
This is my first pull request, please don't hurt me :) |
| @@ -70,7 +70,7 @@ public function getStream() | |||
| */ | |||
| protected function doWrite($message, $newline) | |||
| { | |||
| if (false === @fwrite($this->stream, $message.($newline ? PHP_EOL : ''))) { | |||
| if (false === @fwrite($this->stream, $message) || ($newline && (false === @fwrite($this->stream, PHP_EOL)))) { | |||
There was a problem hiding this comment.
what about:
if (false === @fwrite($this->stream, $message)) {
// should never happen
throw new \RuntimeException('Unable to write output.');
}
if ($newline) {
@fwrite($this->stream, PHP_EOL);
}There was a problem hiding this comment.
In this case no Exception would be thrown if something goes wrong with writing PHP_EOL.
There was a problem hiding this comment.
yep, which won't happen if the first fwrite already succedeed IMHO
There was a problem hiding this comment.
Could be but what speaks against the safe solution?
can you show some statistics about that? |
|
@Dword123 congrats on your first pull request and thanks for contributing to Symfony! The kind 8000 of improvement that you propose here is always tricky for us. This affects the console output stream, which is something that it's not used frequently or on every request. That's why most of the times these improvements don't provide enough gains to be worthy. So let's see what other contributors think about this. |
|
We're using symfony/console over at https://github.com/webfactory/slimdump, a tool that makes SQL dumps to In that case, the If the newline is added to the string before being written, PHP has to allocate additional memory about the size of Asked the other way round: What problem could this change cause? |
|
👍 |
|
Thank you @Dword123. |
This PR was merged into the 2.7 branch. Discussion ---------- Improve memory efficiency | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | This avoids concatenating `$message` and `PHP_EOL` (if necessary) as a new value, greatly improving memory efficiency for large `$message`s. Commits ------- c1df9f2 Improve memory efficiency
This avoids concatenating
$messageandPHP_EOL(if necessary) as a new value, greatly improving memory efficiency for large$messages.