Permalink
Browse files

minor #28200 [Config] Fix slow service discovery for large excluded d…

…irectories (gonzalovilaseca)

This PR was squashed before being merged into the 4.2-dev branch (closes #28200).

Discussion
----------

[Config] Fix slow service discovery for large excluded directories

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  |  no
| BC breaks?    | no
| Deprecations? |  no
| Tests pass?   | no
| Fixed tickets | #26736
| License       | MIT
| Doc PR        |

Not sure if this is a bug fix or not, is more an improvement.
Please for all detail follow the conversation here:
#26736

Commits
-------

fa731e5 [Config] Fix slow service discovery for large excluded directories
  • Loading branch information...
nicolas-grekas committed Oct 23, 2018
2 parents 426cf81 + fa731e5 commit 60394bc348af0fbafdb5ac09ea89cbc877a051ee
@@ -93,7 +93,7 @@ public function import($resource, $type = null, $ignoreErrors = false, $sourceRe
/**
* @internal
*/
protected function glob(string $pattern, bool $recursive, &$resource = null, bool $ignoreErrors = false)
protected function glob(string $pattern, bool $recursive, &$resource = null, bool $ignoreErrors = false, bool $forExclusion = false, array $excluded = array())
{
if (\strlen($pattern) === $i = strcspn($pattern, '*?{[')) {
$prefix = $pattern;
@@ -120,7 +120,7 @@ protected function glob(string $pattern, bool $recursive, &$resource = null, boo
return;
}
$resource = new GlobResource($prefix, $pattern, $recursive);
$resource = new GlobResource($prefix, $pattern, $recursive, $forExclusion, $excluded);
foreach ($resource as $path => $info) {
yield $path => $info;
@@ -27,6 +27,8 @@ class GlobResource implements \IteratorAggregate, SelfCheckingResourceInterface,
private $pattern;
private $recursive;
private $hash;
private $forExclusion;
private $excludedPrefixes;
/**
* @param string $prefix A directory prefix
@@ -35,11 +37,13 @@ class GlobResource implements \IteratorAggregate, SelfCheckingResourceInterface,
*
* @throws \InvalidArgumentException
*/
public function __construct(?string $prefix, string $pattern, bool $recursive)
public function __construct(?string $prefix, string $pattern, bool $recursive, bool $forExclusion = false, array $excludedPrefixes = array())
{
$this->prefix = realpath($prefix) ?: (file_exists($prefix) ? $prefix : false);
$this->pattern = $pattern;
$this->recursive = $recursive;
$this->forExclusion = $forExclusion;
$this->excludedPrefixes = $excludedPrefixes;
if (false === $this->prefix) {
throw new \InvalidArgumentException(sprintf('The path "%s" does not exist.', $prefix));
@@ -95,25 +99,34 @@ public function getIterator()
if (0 !== strpos($this->prefix, 'phar://') && false === strpos($this->pattern, '/**/') && (\defined('GLOB_BRACE') || false === strpos($this->pattern, '{'))) {
foreach (glob($this->prefix.$this->pattern, \defined('GLOB_BRACE') ? GLOB_BRACE : 0) as $path) {
if ($this->recursive && is_dir($path)) {
$files = iterator_to_array(new \RecursiveIteratorIterator(
new \RecursiveCallbackFilterIterator(
new \RecursiveDirectoryIterator($path, \FilesystemIterator::SKIP_DOTS | \FilesystemIterator::FOLLOW_SYMLINKS),
function (\SplFileInfo $file) { return '.' !== $file->getBasename()[0]; }
),
\RecursiveIteratorIterator::LEAVES_ONLY
));
uasort($files, function (\SplFileInfo $a, \SplFileInfo $b) {
return (string) $a > (string) $b ? 1 : -1;
});
foreach ($files as $path => $info) {
if ($info->isFile()) {
yield $path => $info;
if (is_file($path)) {
yield $path => new \SplFileInfo($path);
}
if (!is_dir($path)) {
continue;
}
if ($this->forExclusion) {
yield $path => new \SplFileInfo($path);
continue;
}
if (!$this->recursive || isset($this->excludedPrefixes[str_replace('\\', '/', $path)])) {
continue;
}
$files = iterator_to_array(new \RecursiveIteratorIterator(
new \RecursiveCallbackFilterIterator(
new \RecursiveDirectoryIterator($path, \FilesystemIterator::SKIP_DOTS | \FilesystemIterator::FOLLOW_SYMLINKS),
function (\SplFileInfo $file, $path) {
return !isset($this->excludedPrefixes[str_replace('\\', '/', $path)]) && '.' !== $file->getBasename()[0];
}
),
\RecursiveIteratorIterator::LEAVES_ONLY
));
uasort($files, 'strnatcmp');
foreach ($files as $path => $info) {
if ($info->isFile()) {
yield $path => $info;
}
} elseif (is_file($path)) {
yield $path => new \SplFileInfo($path);
}
}
@@ -132,7 +145,7 @@ function (\SplFileInfo $file) { return '.' !== $file->getBasename()[0]; }
$prefixLen = \strlen($this->prefix);
foreach ($finder->followLinks()->sortByName()->in($this->prefix) as $path => $info) {
if (preg_match($regex, substr('\\' === \DIRECTORY_SEPARATOR ? str_replace('\\', '/', $path) : $path, $prefixLen)) && $info->isFile()) {
if (preg_match($regex, substr(str_replace('\\', '/', $path), $prefixLen)) && $info->isFile()) {
yield $path => $info;
}
}
@@ -47,6 +47,46 @@ public function testIterator()
$this->assertSame($dir, $resource->getPrefix());
}
public function testIteratorForExclusionDoesntIterateThroughSubfolders()
{
$dir = \dirname(__DIR__).\DIRECTORY_SEPARATOR.'Fixtures';
$resource = new GlobResource($dir, \DIRECTORY_SEPARATOR.'Exclude', true, true);
$paths = iterator_to_array($resource);
$file = $dir.\DIRECTORY_SEPARATOR.'Exclude';
$this->assertArrayHasKey($file, $paths);
$this->assertCount(1, $paths);
}
public function testIteratorSkipsFoldersForGivenExcludedPrefixes()
{
$dir = \dirname(__DIR__).\DIRECTORY_SEPARATOR.'Fixtures';
$resource = new GlobResource($dir, '/*Exclude*', true, false, array($dir.\DIRECTORY_SEPARATOR.'Exclude' => true));
$paths = iterator_to_array($resource);
$file = $dir.\DIRECTORY_SEPARATOR.'Exclude'.\DIRECTORY_SEPARATOR.'AnExcludedFile.txt';
$this->assertArrayNotHasKey($file, $paths);
$file = $dir.\DIRECTORY_SEPARATOR.'Exclude'.\DIRECTORY_SEPARATOR.'ExcludeToo'.\DIRECTORY_SEPARATOR.'AnotheExcludedFile.txt';
$this->assertArrayNotHasKey($file, $paths);
}
public function testIteratorSkipsFoldersWithForwardSlashForGivenExcludedPrefixes()
{
$dir = \dirname(__DIR__).\DIRECTORY_SEPARATOR.'Fixtures';
$resource = new GlobResource($dir, '/*Exclude*', true, false, array($dir.'/Exclude' => true));
$paths = iterator_to_array($resource);
$file = $dir.\DIRECTORY_SEPARATOR.'Exclude/AnExcludedFile.txt';
$this->assertArrayNotHasKey($file, $paths);
$file = $dir.\DIRECTORY_SEPARATOR.'Exclude'.\DIRECTORY_SEPARATOR.'ExcludeToo'.\DIRECTORY_SEPARATOR.'AnotheExcludedFile.txt';
$this->assertArrayNotHasKey($file, $paths);
}
public function testIsFreshNonRecursiveDetectsNewFile()
{
$dir = \dirname(__DIR__).'/Fixtures';
@@ -109,7 +109,7 @@ private function findClasses($namespace, $pattern, array $excludePatterns)
$excludePrefix = null;
$excludePatterns = $parameterBag->unescapeValue($parameterBag->resolveValue($excludePatterns));
foreach ($excludePatterns as $excludePattern) {
foreach ($this->glob($excludePattern, true, $resource) as $path => $info) {
foreach ($this->glob($excludePattern, true, $resource, false, true) as $path => $info) {
if (null === $excludePrefix) {
$excludePrefix = $resource->getPrefix();
}
@@ -123,7 +123,7 @@ private function findClasses($namespace, $pattern, array $excludePatterns)
$classes = array();
$extRegexp = '/\\.php$/';
$prefixLen = null;
foreach ($this->glob($pattern, true, $resource) as $path => $info) {
foreach ($this->glob($pattern, true, $resource, false, false, $excludePaths) as $path => $info) {
if (null === $prefixLen) {
$prefixLen = \strlen($resource->getPrefix());
@@ -612,7 +612,19 @@ public function testPrototype()
$fixturesDir = \dirname(__DIR__).\DIRECTORY_SEPARATOR.'Fixtures'.\DIRECTORY_SEPARATOR;
$this->assertTrue(false !== array_search(new FileResource($fixturesDir.'xml'.\DIRECTORY_SEPARATOR.'services_prototype.xml'), $resources));
$this->assertTrue(false !== array_search(new GlobResource($fixturesDir.'Prototype', '/*', true), $resources));
$prototypeRealPath = \realpath(__DIR__.\DIRECTORY_SEPARATOR.'..'.\DIRECTORY_SEPARATOR.'Fixtures'.\DIRECTORY_SEPARATOR.'Prototype');
$globResource = new GlobResource(
$fixturesDir.'Prototype',
'/*',
true,
false,
array(
str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'OtherDir') => true,
str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'BadClasses') => true,
)
);
$this->assertTrue(false !== array_search($globResource, $resources));
$resources = array_map('strval', $resources);
$this->assertContains('reflection.Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo', $resources);
$this->assertContains('reflection.Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar', $resources);
@@ -632,7 +644,19 @@ public function testPrototypeExcludeWithArray()
$fixturesDir = \dirname(__DIR__).\DIRECTORY_SEPARATOR.'Fixtures'.\DIRECTORY_SEPARATOR;
$this->assertTrue(false !== array_search(new FileResource($fixturesDir.'xml'.\DIRECTORY_SEPARATOR.'services_prototype_array.xml'), $resources));
$this->assertTrue(false !== array_search(new GlobResource($fixturesDir.'Prototype', '/*', true), $resources));
$prototypeRealPath = realpath(__DIR__.\DIRECTORY_SEPARATOR.'..'.\DIRECTORY_SEPARATOR.'Fixtures'.\DIRECTORY_SEPARATOR.'Prototype');
$globResource = new GlobResource(
$fixturesDir.'Prototype',
'/*',
true,
false,
array(
str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'BadClasses') => true,
str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'OtherDir') => true,
)
);
$this->assertTrue(false !== array_search($globResource, $resources));
$resources = array_map('strval', $resources);
$this->assertContains('reflection.Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo', $resources);
$this->assertContains('reflection.Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar', $resources);
@@ -365,7 +365,18 @@ public function testPrototype()
$fixturesDir = \dirname(__DIR__).\DIRECTORY_SEPARATOR.'Fixtures'.\DIRECTORY_SEPARATOR;
$this->assertTrue(false !== array_search(new FileResource($fixturesDir.'yaml'.\DIRECTORY_SEPARATOR.'services_prototype.yml'), $resources));
$this->assertTrue(false !== array_search(new GlobResource($fixturesDir.'Prototype', '', true), $resources));
$prototypeRealPath = \realpath(__DIR__.\DIRECTORY_SEPARATOR.'..'.\DIRECTORY_SEPARATOR.'Fixtures'.\DIRECTORY_SEPARATOR.'Prototype');
$globResource = new GlobResource(
$fixturesDir.'Prototype',
'',
true,
false, array(
str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'BadClasses') => true,
str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'OtherDir') => true,
)
);
$this->assertTrue(false !== array_search($globResource, $resources));
$resources = array_map('strval', $resources);
$this->assertContains('reflection.Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo', $resources);
$this->assertContains('reflection.Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar', $resources);

0 comments on commit 60394bc

Please sign in to comment.