minor #28200 [Config] Fix slow service discovery for large excluded directories (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:
https://github.com/symfony/symfony/issues/26736
Commits
-------
fa731e53e9
[Config] Fix slow service discovery for large excluded directories
This commit is contained in:
commit
60394bc348
@ -93,7 +93,7 @@ abstract class FileLoader extends Loader
|
|||||||
/**
|
/**
|
||||||
* @internal
|
* @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, '*?{[')) {
|
if (\strlen($pattern) === $i = strcspn($pattern, '*?{[')) {
|
||||||
$prefix = $pattern;
|
$prefix = $pattern;
|
||||||
@ -120,7 +120,7 @@ abstract class FileLoader extends Loader
|
|||||||
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
$resource = new GlobResource($prefix, $pattern, $recursive);
|
$resource = new GlobResource($prefix, $pattern, $recursive, $forExclusion, $excluded);
|
||||||
|
|
||||||
foreach ($resource as $path => $info) {
|
foreach ($resource as $path => $info) {
|
||||||
yield $path => $info;
|
yield $path => $info;
|
||||||
|
@ -27,6 +27,8 @@ class GlobResource implements \IteratorAggregate, SelfCheckingResourceInterface,
|
|||||||
private $pattern;
|
private $pattern;
|
||||||
private $recursive;
|
private $recursive;
|
||||||
private $hash;
|
private $hash;
|
||||||
|
private $forExclusion;
|
||||||
|
private $excludedPrefixes;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @param string $prefix A directory prefix
|
* @param string $prefix A directory prefix
|
||||||
@ -35,11 +37,13 @@ class GlobResource implements \IteratorAggregate, SelfCheckingResourceInterface,
|
|||||||
*
|
*
|
||||||
* @throws \InvalidArgumentException
|
* @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->prefix = realpath($prefix) ?: (file_exists($prefix) ? $prefix : false);
|
||||||
$this->pattern = $pattern;
|
$this->pattern = $pattern;
|
||||||
$this->recursive = $recursive;
|
$this->recursive = $recursive;
|
||||||
|
$this->forExclusion = $forExclusion;
|
||||||
|
$this->excludedPrefixes = $excludedPrefixes;
|
||||||
|
|
||||||
if (false === $this->prefix) {
|
if (false === $this->prefix) {
|
||||||
throw new \InvalidArgumentException(sprintf('The path "%s" does not exist.', $prefix));
|
throw new \InvalidArgumentException(sprintf('The path "%s" does not exist.', $prefix));
|
||||||
@ -95,26 +99,35 @@ class GlobResource implements \IteratorAggregate, SelfCheckingResourceInterface,
|
|||||||
|
|
||||||
if (0 !== strpos($this->prefix, 'phar://') && false === strpos($this->pattern, '/**/') && (\defined('GLOB_BRACE') || false === strpos($this->pattern, '{'))) {
|
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) {
|
foreach (glob($this->prefix.$this->pattern, \defined('GLOB_BRACE') ? GLOB_BRACE : 0) as $path) {
|
||||||
if ($this->recursive && is_dir($path)) {
|
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(
|
$files = iterator_to_array(new \RecursiveIteratorIterator(
|
||||||
new \RecursiveCallbackFilterIterator(
|
new \RecursiveCallbackFilterIterator(
|
||||||
new \RecursiveDirectoryIterator($path, \FilesystemIterator::SKIP_DOTS | \FilesystemIterator::FOLLOW_SYMLINKS),
|
new \RecursiveDirectoryIterator($path, \FilesystemIterator::SKIP_DOTS | \FilesystemIterator::FOLLOW_SYMLINKS),
|
||||||
function (\SplFileInfo $file) { return '.' !== $file->getBasename()[0]; }
|
function (\SplFileInfo $file, $path) {
|
||||||
|
return !isset($this->excludedPrefixes[str_replace('\\', '/', $path)]) && '.' !== $file->getBasename()[0];
|
||||||
|
}
|
||||||
),
|
),
|
||||||
\RecursiveIteratorIterator::LEAVES_ONLY
|
\RecursiveIteratorIterator::LEAVES_ONLY
|
||||||
));
|
));
|
||||||
uasort($files, function (\SplFileInfo $a, \SplFileInfo $b) {
|
uasort($files, 'strnatcmp');
|
||||||
return (string) $a > (string) $b ? 1 : -1;
|
|
||||||
});
|
|
||||||
|
|
||||||
foreach ($files as $path => $info) {
|
foreach ($files as $path => $info) {
|
||||||
if ($info->isFile()) {
|
if ($info->isFile()) {
|
||||||
yield $path => $info;
|
yield $path => $info;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} elseif (is_file($path)) {
|
|
||||||
yield $path => new \SplFileInfo($path);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return;
|
return;
|
||||||
@ -132,7 +145,7 @@ class GlobResource implements \IteratorAggregate, SelfCheckingResourceInterface,
|
|||||||
|
|
||||||
$prefixLen = \strlen($this->prefix);
|
$prefixLen = \strlen($this->prefix);
|
||||||
foreach ($finder->followLinks()->sortByName()->in($this->prefix) as $path => $info) {
|
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;
|
yield $path => $info;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -47,6 +47,46 @@ class GlobResourceTest extends TestCase
|
|||||||
$this->assertSame($dir, $resource->getPrefix());
|
$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()
|
public function testIsFreshNonRecursiveDetectsNewFile()
|
||||||
{
|
{
|
||||||
$dir = \dirname(__DIR__).'/Fixtures';
|
$dir = \dirname(__DIR__).'/Fixtures';
|
||||||
|
@ -109,7 +109,7 @@ abstract class FileLoader extends BaseFileLoader
|
|||||||
$excludePrefix = null;
|
$excludePrefix = null;
|
||||||
$excludePatterns = $parameterBag->unescapeValue($parameterBag->resolveValue($excludePatterns));
|
$excludePatterns = $parameterBag->unescapeValue($parameterBag->resolveValue($excludePatterns));
|
||||||
foreach ($excludePatterns as $excludePattern) {
|
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) {
|
if (null === $excludePrefix) {
|
||||||
$excludePrefix = $resource->getPrefix();
|
$excludePrefix = $resource->getPrefix();
|
||||||
}
|
}
|
||||||
@ -123,7 +123,7 @@ abstract class FileLoader extends BaseFileLoader
|
|||||||
$classes = array();
|
$classes = array();
|
||||||
$extRegexp = '/\\.php$/';
|
$extRegexp = '/\\.php$/';
|
||||||
$prefixLen = null;
|
$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) {
|
if (null === $prefixLen) {
|
||||||
$prefixLen = \strlen($resource->getPrefix());
|
$prefixLen = \strlen($resource->getPrefix());
|
||||||
|
|
||||||
|
@ -612,7 +612,19 @@ class XmlFileLoaderTest extends TestCase
|
|||||||
|
|
||||||
$fixturesDir = \dirname(__DIR__).\DIRECTORY_SEPARATOR.'Fixtures'.\DIRECTORY_SEPARATOR;
|
$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 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);
|
$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\Foo', $resources);
|
||||||
$this->assertContains('reflection.Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar', $resources);
|
$this->assertContains('reflection.Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar', $resources);
|
||||||
@ -632,7 +644,19 @@ class XmlFileLoaderTest extends TestCase
|
|||||||
|
|
||||||
$fixturesDir = \dirname(__DIR__).\DIRECTORY_SEPARATOR.'Fixtures'.\DIRECTORY_SEPARATOR;
|
$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 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);
|
$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\Foo', $resources);
|
||||||
$this->assertContains('reflection.Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar', $resources);
|
$this->assertContains('reflection.Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar', $resources);
|
||||||
|
@ -365,7 +365,18 @@ class YamlFileLoaderTest extends TestCase
|
|||||||
|
|
||||||
$fixturesDir = \dirname(__DIR__).\DIRECTORY_SEPARATOR.'Fixtures'.\DIRECTORY_SEPARATOR;
|
$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 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);
|
$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\Foo', $resources);
|
||||||
$this->assertContains('reflection.Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar', $resources);
|
$this->assertContains('reflection.Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar', $resources);
|
||||||
|
Reference in New Issue
Block a user