Skip to content

Commit

Permalink
BAP-14658: LoggerDataCollector called over 500k times in dev mode (#1…
Browse files Browse the repository at this point in the history
…1598)

- added local cache for a logger collector
- overwrite symfony`s data_collector.logger class
- added tests
  • Loading branch information
vtsykun authored and Mykhailo Sulyma committed Jul 11, 2017
1 parent 9e75789 commit 915723a
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 0 deletions.
26 changes: 26 additions & 0 deletions src/Oro/Bundle/LoggerBundle/DataCollector/LoggerDataCollector.php
@@ -0,0 +1,26 @@
<?php

namespace Oro\Bundle\LoggerBundle\DataCollector;

This comment has been minimized.

Copy link
@vtsykun

vtsykun Jul 26, 2017

Author Contributor

Should be remove after update symfony version 3.3+ and merge PR
symfony/symfony#23683
symfony/symfony#23659
\cc @manowark

This comment has been minimized.

Copy link
@manowark

manowark Jul 26, 2017

Contributor

@vtsykun,
great, thx.


use Symfony\Component\HttpKernel\DataCollector\LoggerDataCollector as BaseDataCollector;

class LoggerDataCollector extends BaseDataCollector
{
/**
* {@inheritdoc}
*/
public function lateCollect()
{
// On event kernel.terminate ProfilerListener save profile for each sub-request and calls method lateCollect,
// but sets of logs for each sub-request and master-requests are same,
// so here added local caching to prevent work on the same data and fix performance in dev mode
static $localCache;

if ($localCache === null) {
parent::lateCollect();
$localCache = $this->data;
} else {
$this->data = $localCache;
}
}
}
@@ -0,0 +1,21 @@
<?php

namespace Oro\Bundle\LoggerBundle\DependencyInjection\Compiler;

use Oro\Bundle\LoggerBundle\DataCollector\LoggerDataCollector;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;

class LoggerCollectorPass implements CompilerPassInterface
{
/**
* {@inheritdoc}
*/
public function process(ContainerBuilder $container)
{
if ($container->hasDefinition('data_collector.logger')) {
$definition = $container->getDefinition('data_collector.logger');
$definition->setClass(LoggerDataCollector::class);
}
}
}
2 changes: 2 additions & 0 deletions src/Oro/Bundle/LoggerBundle/OroLoggerBundle.php
Expand Up @@ -2,6 +2,7 @@

namespace Oro\Bundle\LoggerBundle;

use Oro\Bundle\LoggerBundle\DependencyInjection\Compiler\LoggerCollectorPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\HttpKernel\Bundle\Bundle;

Expand All @@ -19,5 +20,6 @@ public function build(ContainerBuilder $container)

$container->addCompilerPass(new DetailedLogsHandlerPass());
$container->addCompilerPass(new SwiftMailerHandlerPass());
$container->addCompilerPass(new LoggerCollectorPass());
}
}
@@ -0,0 +1,23 @@
<?php

namespace Oro\Bundle\LoggerBundle\Tests\Unit\DataCollector;

use Oro\Bundle\LoggerBundle\DataCollector\LoggerDataCollector;
use Symfony\Component\HttpKernel\Log\DebugLoggerInterface;

class LoggerDataCollectorTest extends \PHPUnit_Framework_TestCase
{
public function testLateCollect()
{
$logger = $this->createMock(DebugLoggerInterface::class);
$logger->expects($this->once())
->method('countErrors');
$logger->expects($this->any())
->method('getLogs')
->willReturn([]);

$collector = new LoggerDataCollector($logger);
$collector->lateCollect();
$collector->lateCollect();
}
}
@@ -0,0 +1,51 @@
<?php

namespace Oro\Bundle\LoggerBundle\Tests\Unit\DependencyInjection\Compiler;

use Oro\Bundle\LoggerBundle\DependencyInjection\Compiler\LoggerCollectorPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;

class LoggerCollectorPassTest extends \PHPUnit_Framework_TestCase
{
/**
* @var LoggerCollectorPass
*/
protected $compilerPass;

/**
* {@inheritdoc}
*/
protected function setUp()
{
$this->compilerPass = new LoggerCollectorPass();
}

public function testProcessWhenServiceNotExist()
{
/** @var \PHPUnit_Framework_MockObject_MockObject|ContainerBuilder $containerBuilder */
$containerBuilder = $this->createMock(ContainerBuilder::class);
$containerBuilder->expects($this->any())
->method('hasDefinition')
->with('data_collector.logger')
->willReturn(false);
$containerBuilder->expects($this->never())->method('getDefinition');

$this->compilerPass->process($containerBuilder);
}

public function testProcessWhenServiceExist()
{
/** @var \PHPUnit_Framework_MockObject_MockObject|ContainerBuilder $containerBuilder */
$containerBuilder = $this->createMock(ContainerBuilder::class);
$containerBuilder->expects($this->any())
->method('hasDefinition')
->with('data_collector.logger')
->willReturn(true);
$containerBuilder->expects($this->once())
->method('getDefinition')
->willReturn(new Definition());

$this->compilerPass->process($containerBuilder);
}
}

0 comments on commit 915723a

Please sign in to comment.