Use NullLogger when available#14682
Use NullLogger when available#14682ogizanagi wants to merge 1 commit intosymfony:2.3from ogizanagi:null_logger/2.3
Conversation
|
The idea behind the current implementation was speed. |
|
Some insight about performance: https://blackfire.io/profiles/compare/a5e26195-64ad-4b99-bf3d-e03a2ed4a3c0/graph Code: <?php
require __DIR__.'/vendor/autoload.php';
use Psr\Log\LoggerInterface;
use Psr\Log\NullLogger;
class Before
{
public function __construct(LoggerInterface $logger = null)
{
$this->logger = $logger;
}
public function doSomething()
{
if ($this->logger) {
$this->logger->debug('we are doing something.');
}
$a = 1;
$b = 1;
$c = $a + $b;
if ($this->logger) {
$this->logger->debug('we have done something.');
}
return $c;
}
}
class After
{
public function __construct(LoggerInterface $logger = null)
{
$this->logger = $logger ?: new NullLogger;
}
public function doSomething()
{
$this->logger->debug('we are doing something.');
$a = 1;
$b = 1;
$c = $a + $b;
$this->logger->debug('we have done something.');
return $c;
}
}
$logger = new NullLogger;
for ($i=0; $i < 1000; $i++) {
// $s = new Before();
$s = new After();
$s->doSomething();
} |
|
@lyrixx hehe, i've done something similar, but without creating object in a loop <?php
require_once __DIR__.'/vendor/autoload.php';
class Foo
{
public $logger = null;
public function __construct($logger = null)
{
$this->logger = $logger;
}
public function call()
{
if (null !== $this->logger) {
$this->logger->notice('hello');
}
}
}
class Bar
{
public $logger = null;
public function __construct($logger = null)
{
$this->logger = $logger ?: new \Psr\Log\NullLogger();
}
public function call()
{
$this->logger->notice('hello');
}
}
$name = isset($argv[1]) ? $argv[1] : null;
switch ($name) {
case 'foo':
$foo = new Foo();
break;
case 'fool':
$foo = new Foo(new \Monolog\Logger('foo'));
break;
case 'bar':
$foo = new Bar();
break;
case 'barl':
$foo = new Bar(new \Monolog\Logger('foo'));
break;
default:
throw new \InvalidArgumentException('Accepted arguments: foo|fool|bar|barl');
}
for ($i = 0; $i < 100000; $i++) {
$foo->call();
} |
|
Alright, let's close this, then :) |
|
Can we rethink the results provided by the benchmarks? In the data shown by @jakzal the performance degrades 600ms when logging 100,000 times. How many log calls do you get in development env? ~200 at most? And in production? Most of the requests log nothing or less than 10 lines. Applying the same results shown by @jakzal, logging 100 lines would add 0.6ms overhead. |
|
@javiereguiluz Once upon a time, you shared an (issue|article|blog post) about lots of micro optimisations that make SQLite much much faster. I think here is a good example on how to optimize symfony. And so, finally, even if I use this "hack" in my own code base, now I think it's better to keep the current code in symfony for performance reason. |
|
@lyrixx yes, but please let's keep in mind for the future that if we make 100,000 iterations of a new proposed feature in a benchmark, the result is probably going to be much slower. |
|
I don't see why we are even talking about this topic again. That's a non issue, changing the current way does not fix anything and AFAIK, it does not bring any new features. |
|
The only debate remaining to me is: does it prevent us to use this pattern for new features ? (For consistency reasons) |
|
IMO, Yes. NullLogger is not for the framework for performance reasons. |

Use
Psr\Log\NullLoggerwhen available in order to avoid littering code withif ($this->logger) { }blocks.Changes were made only in component and bundles where
psr\logis declared as a dependency incomposer.json, or at leastsymfony/http-kernelwhich requires it.If this is accepted for merging, some changes might remain to do in newest branches.