8000 [Config] Fix slow service discovery for large excluded directories · symfony/symfony@fa731e5 · GitHub
[go: up one dir, main page]
Skip to content

Commit fa731e5

Browse files
gonzalovilasecanicolas-grekas
gonzalovilaseca
authored and
nicolas-grekas
committed
[Config] Fix slow service discovery for large excluded directories
1 parent 15aa25a commit fa731e5

File tree

8 files changed

+114
-26
lines changed

8 files changed

+114
-26
lines changed

‎src/Symfony/Component/Config/Loader/FileLoader.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public function import($resource, $type = null, $ignoreErrors = false, $sourceRe
9393
/**
9494
* @internal
9595
*/
96-
protected function glob(string $pattern, bool $recursive, &$resource = null, bool $ignoreErrors = false)
96+
protected function glob(string $pattern, bool $recursive, &$resource = null, bool $ignoreErrors = false, bool $forExclusion = false, array $excluded = array())
9797
{
9898
if (\strlen($pattern) === $i = strcspn($pattern, '*?{[')) {
9999
$prefix = $pattern;
@@ -120,7 +120,7 @@ protected function glob(string $pattern, bool $recursive, &$resource = null, boo
120120

121121
return;
122122
}
123-
$resource = new GlobResource($prefix, $pattern, $recursive);
123+
$resource = new GlobResource($prefix, $pattern, $recursive, $forExclusion, $excluded);
124124

125125
foreach ($resource as $path => $info) {
126126
yield $path => $info;

‎src/Symfony/Component/Config/Resource/GlobResource.php

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ class GlobResource implements \IteratorAggregate, SelfCheckingResourceInterface,
2727
private $pattern;
2828
private $recursive;
2929
private $hash;
30+
private $forExclusion;
31+
private $excludedPrefixes;
3032

3133
/**
3234
* @param string $prefix A directory prefix
@@ -35,11 +37,13 @@ class GlobResource implements \IteratorAggregate, SelfCheckingResourceInterface,
3537
*
3638
* @throws \InvalidArgumentException
3739
*/
38-
public function __construct(?string $prefix, string $pattern, bool $recursive)
40+
public function __construct(?string $prefix, string $pattern, bool $recursive, bool $forExclusion = false, array $excludedPrefixes = array())
3941
{
4042
$this->prefix = realpath($prefix) ?: (file_exists($prefix) ? $prefix : false);
4143
$this->pattern = $pattern;
4244
$this->recursive = $recursive;
45+
$this->forExclusion = $forExclusion;
46+
$this->excludedPrefixes = $excludedPrefixes;
4347

4448
if (false === $this->prefix) {
4549
throw new \InvalidArgumentException(sprintf('The path "%s" does not exist.', $prefix));
@@ -95,25 +99,34 @@ public function getIterator()
9599

96100
if (0 !== strpos($this->prefix, 'phar://') && false === strpos($this->pattern, '/**/') && (\defined('GLOB_BRACE') || false === strpos($this->pattern, '{'))) {
97101
foreach (glob($this->prefix.$this->pattern, \defined('GLOB_BRACE') ? GLOB_BRACE : 0) as $path) {
98-
if ($this->recursive && is_dir($path)) {
99-
$files = iterator_to_array(new \RecursiveIteratorIterator(
100-
new \RecursiveCallbackFilterIterator(
101-
new \RecursiveDirectoryIterator($path, \FilesystemIterator::SKIP_DOTS | \FilesystemIterator::FOLLOW_SYMLINKS),
102-
function (\SplFileInfo $file) { return '.' !== $file->getBasename()[0]; }
103-
),
104-
\RecursiveIteratorIterator::LEAVES_ONLY
105-
));
106-
uasort($files, function (\SplFileInfo $a, \SplFileInfo $b) {
107-
return (string) $a > (string) $b ? 1 : -1;
108-
});
109-
110-
foreach ($files as $path => $info) {
111-
if ($info->isFile()) {
112-
yield $path => $info;
102+
if (is_file($path)) {
103+
yield $path => new \SplFileInfo($path);
104+
}
105+
if (!is_dir($path)) {
106+
continue;
107+
}
108+
if ($this->forExclusion) {
109+
yield $path => new \SplFileInfo($path);
110+
continue;
111+
}
112+
if (!$this->recursive || isset($this->excludedPrefixes[str_replace('\\', '/', $path)])) {
113+
continue;
114+
}
115+
$files = iterator_to_array(new \RecursiveIteratorIterator(
116+
new \RecursiveCallbackFilterIterator(
117+
new \RecursiveDirectoryIterator($path, \FilesystemIterator::SKIP_DOTS | \FilesystemIterator::FOLLOW_SYMLINKS),
118+
function (\SplFileInfo $file, $path) {
119+
return !isset($this->excludedPrefixes[str_replace('\\', '/', $path)]) && '.' !== $file->getBasename()[0];
113120
}
121+
),
122+
\RecursiveIteratorIterator::LEAVES_ONLY
123+
));
124+
uasort($files, 'strnatcmp');
125+
126+
foreach ($files as $path => $info) {
127+
if ($info->isFile()) {
128+
yield $path => $info;
114129
}
115-
} elseif (is_file($path)) {
116-
yield $path => new \SplFileInfo($path);
117130
}
118131
}
119132

@@ -132,7 +145,7 @@ function (\SplFileInfo $file) { return '.' !== $file->getBasename()[0]; }
132145

133146
$prefixLen = \strlen($this->prefix);
134147
foreach ($finder->followLinks()->sortByName()->in($this->prefix) as $path => $info) {
135-
if (preg_match($regex, substr('\\' === \DIRECTORY_SEPARATOR ? str_replace('\\', '/', $path) : $path, $prefixLen)) && $info->isFile()) {
148+
if (preg_match($regex, substr(str_replace('\\', '/', $path), $prefixLen)) && $info->isFile()) {
136149
yield $path => $info;
137150
}
138151
}

‎src/Symfony/Component/Config/Tests/Fixtures/Exclude/AnExcludedFile.txt

Whitespace-only changes.

‎src/Symfony/Component/Config/Tests/Fixtures/Exclude/ExcludeToo/AnotheExcludedFile.txt

Whitespace-only changes.

‎src/Symfony/Component/Config/Tests/Resource/GlobResourceTest.php

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,46 @@ public function testIterator()
4747
$this->assertSame($dir, $resource->getPrefix());
4848
}
4949

50+
public function testIteratorForExclusionDoesntIterateThroughSubfolders()
51+
{
52+
$dir = \dirname(__DIR__).\DIRECTORY_SEPARATOR.'Fixtures';
53+
$resource = new GlobResource($dir, \DIRECTORY_SEPARATOR.'Exclude', true, true);
54+
55+
$paths = iterator_to_array($resource);
56+
57+
$file = $dir.\DIRECTORY_SEPARATOR.'Exclude';
58+
$this->assertArrayHasKey($file, $paths);
59+
$this->assertCount(1, $paths);
60+
}
61+
62+
public function testIteratorSkipsFoldersForGivenExcludedPrefixes()
63+
{
64+
$dir = \dirname(__DIR__).\DIRECTORY_SEPARATOR.'Fixtures';
65+
$resource = new GlobResource($dir, '/*Exclude*', true, false, array($dir.\DIRECTORY_SEPARATOR.'Exclude' => true));
66+
67+
$paths = iterator_to_array($resource);
68+
69+
$file = $dir.\DIRECTORY_SEPARATOR.'Exclude'.\DIRECTORY_SEPARATOR.'AnExcludedFile.txt';
70+
$this->assertArrayNotHasKey($file, $paths);
71+
72+
$file = $dir.\DIRECTORY_SEPARATOR.'Exclude'.\DIRECTORY_SEPARATOR.'ExcludeToo'.\DIRECTORY_SEPARATOR.'AnotheExcludedFile.txt';
73+
$this->assertArrayNotHasKey($file, $paths);
74+
}
75+
76+
public function testIteratorSkipsFoldersWithForwardSlashForGivenExcludedPrefixes()
77+
{
78+
$dir = \dirname(__DIR__).\DIRECTORY_SEPARATOR.'Fixtures';
79+
$resource = new GlobResource($dir, '/*Exclude*', true, false, array($dir.'/Exclude' => true));
80+
81+
$paths = iterator_to_array($resource);
82+
83+
$file = $dir.\DIRECTORY_SEPARATOR.'Exclude/AnExcludedFile.txt';
84+
$this->assertArrayNotHasKey($file, $paths);
85+
86+
$file = $dir.\DIRECTORY_SEPARATOR.'Exclude'.\DIRECTORY_SEPARATOR.'ExcludeToo'.\DIRECTORY_SEPARATOR.'AnotheExcludedFile.txt';
87+
$this->assertArrayNotHasKey($file, $paths);
88+
}
89+
5090
public function testIsFreshNonRecursiveDetectsNewFile()
5191
{
5292
$dir = \dirname(__DIR__).'/Fixtures';

‎src/Symfony/Component/DependencyInjection/Loader/FileLoader.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ private function findClasses($namespace, $pattern, array $excludePatterns)
109109
$excludePrefix = null;
110110
$excludePatterns = $parameterBag->unescapeValue($parameterBag->resolveValue($excludePatterns));
111111
foreach ($excludePatterns as $excludePattern) {
112-
foreach ($this->glob($excludePattern, true, $resource) as $path => $info) {
112+
foreach ($this->glob($excludePattern, true, $resource, false, true) as $path => $info) {
113113
if (null === $excludePrefix) {
114114
$excludePrefix = $resource->getPrefix();
115115
}
@@ -123,7 +123,7 @@ private function findClasses($namespace, $pattern, array $excludePatterns)
123123
$classes = array();
124124
$extRegexp = '/\\.php$/';
125125
$prefixLen = null;
126-
foreach ($this->glob($pattern, true, $resource) as $path => $info) {
126+
foreach ($this->glob($pattern, true, $resource, false, false, $excludePaths) as $path => $info) {
127127
if (null === $prefixLen) {
128128
$prefixLen = \strlen($resource->getPrefix());
129129

‎src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -611,7 +611,19 @@ public function testPrototype()
611611

612612
$fixturesDir = \dirname(__DIR__).\DIRECTORY_SEPARATOR.'Fixtures'.\DIRECTORY_SEPARATOR;
613613
$this->assertTrue(false !== array_search(new FileResource($fixturesDir.'xml'.\DIRECTORY_SEPARATOR.'services_prototype.xml'), $resources));
614-
$this->assertTrue(false !== array_search(new GlobResource($fixturesDir.'Prototype', '/*', true), $resources));
614+
615+
$prototypeRealPath = \realpath(__DIR__.\DIRECTORY_SEPARATOR.'..'.\DIRECTORY_SEPARATOR.'Fixtures'.\DIRECTORY_SEPARATOR.'Prototype');
616+
$globResource = new GlobResource(
617+
$fixturesDir.'Prototype',
618+
'/*',
619+
true,
620+
false,
621+
array(
622+
str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'OtherDir') => true,
623+
str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'BadClasses') => true,
624+
)
625+
);
626+
$this->assertTrue(false !== array_search($globResource, $resources));
615627
$resources = array_map('strval', $resources);
616628
$this->assertContains('reflection.Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo', $resources);
617629
$this->assertContains('reflection.Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar', $resources);
@@ -631,7 +643,19 @@ public function testPrototypeExcludeWithArray()
631643

632644
$fixturesDir = \dirname(__DIR__).\DIRECTORY_SEPARATOR.'Fixtures'.\DIRECTORY_SEPARATOR;
633645
$this->assertTrue(false !== array_search(new FileResource($fixturesDir.'xml'.\DIRECTORY_SEPARATOR.'services_prototype_array.xml'), $resources));
634-
$this->assertTrue(false !== array_search(new GlobResource($fixturesDir.'Prototype', '/*', true), $resources));
646+
647+
$prototypeRealPath = realpath(__DIR__.\DIRECTORY_SEPARATOR.'..'.\DIRECTORY_SEPARATOR.'Fixtures'.\DIRECTORY_SEPARATOR.'Prototype');
648+
$globResource = new GlobResource(
649+
$fixturesDir.'Prototype',
650+
'/*',
651+
true,
652+
false,
653+
array(
654+
str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'BadClasses') => true,
655+
str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'OtherDir') => true,
656+
)
657+
);
658+
$this->assertTrue(false !== array_search($globResource, $resources));
635659
$resources = array_map('strval', $resources);
636660
$this->assertContains('reflection.Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo', $resources);
637661
$this->assertContains('reflection.Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar', $resources);

‎src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,18 @@ public function testPrototype()
365365

366366
$fixturesDir = \dirname(__DIR__).\DIRECTORY_SEPARATOR.'Fixtures'.\DIRECTORY_SEPARATOR;
367367
$this->assertTrue(false !== array_search(new FileResource($fixturesDir.'yaml'.\DIRECTORY_SEPARATOR.'services_prototype.yml'), $resources));
368-
$this->assertTrue(false !== array_search(new GlobResource($fixturesDir.'Prototype', '', true), $resources));
368+
369+
$prototypeRealPath = \realpath(__DIR__.\DIRECTORY_SEPARATOR.'..'.\DIRECTORY_SEPARATOR.'Fixtures'.\DIRECTORY_SEPARATOR.'Prototype');
370+
$globResource = new GlobResource(
371+
$fixturesDir.'Prototype',
372+
'',
373+
true,
374+
false, array(
375+
str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'BadClasses') => true,
376+
str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'OtherDir') => true,
377+
)
378+
);
379+
$this->assertTrue(false !== array_search($globResource, $resources));
369380
$resources = array_map('strval', $resources);
370381
$this->assertContains('reflection.Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo', $resources);
371382
$this->assertContains('reflection.Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar', $resources);

0 commit comments

Comments
 (0)
0