8000 bug #15826 [Finder] Optimize the hot-path (nicolas-grekas) · symfony/symfony@4e7644c · GitHub
[go: up one dir, main page]
8000
Skip to content

Commit 4e7644c

Browse files
fabpotfabpot
committed
bug #15826 [Finder] Optimize the hot-path (nicolas-grekas)
This PR was merged into the 2.3 branch. Discussion ---------- [Finder] Optimize the hot-path | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #15824 | License | MIT | Doc PR | - A significant part of the perf gain in #15802 was related to filters not being applied recursively... #15824 fixing this, performance dropped again. This PR optimizes the hot path by replacing a regexp test by a simple `isset` when possible. Blackfire diff after #15824 is the following: https://blackfire.io/profiles/compare/9e489018-998d-4acb-92a0-46011828e83b/graph `preg_match` is not called anymore, and `Symfony\Component\Finder\Iterator\RecursiveDirectoryIterator::current()` is also cut by two. When this `isset` optimization is disabled and replaced by a concatenation of all the regexps patterns in a single bigger one, the gain is still significant but lower: https://blackfire.io/profiles/compare/db86b80e-b63e-4fc9-9ff3-9ed32baeb948/graph This makes me think that an other and last round of optimization is possible by merging all regexps in one in MultiplePcreFilterIterator filters. If someone wants to work on this, please do it :) Commits ------- f156de6 [Finder] Optimize the hot-path
2 parents 4a9676b + f156de6 commit 4e7644c

File tree

2 files changed

+50
-13
lines changed

2 files changed

+50
-13
lines changed

‎src/Symfony/Component/Finder/Iterator/ExcludeDirectoryFilterIterator.php

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,10 @@
1818
*/
1919
class ExcludeDirectoryFilterIterator extends FilterIterator implements \RecursiveIterator
2020
{
21+
private $iterator;
2122
private $isRecursive;
22-
private $patterns;
23+
private $excludedDirs = array();
24+
private $excludedPattern;
2325

2426
/**
2527
* Constructor.
@@ -29,10 +31,18 @@ class ExcludeDirectoryFilterIterator extends FilterIterator implements \Recursiv
2931
*/
3032
public function __construct(\Iterator $iterator, array $directories)
3133
{
34+
$this->iterator = $iterator;
3235
$this->isRecursive = $iterator instanceof \RecursiveIterator;
33-
$this->patterns = array();
36+
$patterns = array();
3437
foreach ($directories as $directory) {
35-
$this->patterns[] = '#(^|/)'.preg_quote($directory, '#').'(/|$)#';
38+
if (!$this->isRecursive || false !== strpos($directory, '/')) {
39+
$patterns[] = preg_quote($directory, '#');
40+
} else {
41+
$this->excludedDirs[$directory] = true;
42+
}
43+
}
44+
if ($patterns) {
45+
$this->excludedPattern = '#(?:^|/)(?:'.implode('|', $patterns).')(?:/|$)#';
3646
}
3747

3848
parent::__construct($iterator);
@@ -45,26 +55,30 @@ public function __construct(\Iterator $iterator, array $directories)
4555
*/
4656
public function accept()
4757
{
48-
$path = $this->isDir() ? $this->current()->getRelativePathname() : $this->current()->getRelativePath();
49-
$path = str_replace('\\', '/', $path);
50-
foreach ($this->patterns as $pattern) {
51-
if (preg_match($pattern, $path)) {
52-
return false;
53-
}
58+
if ($this->isRecursive && isset($this->excludedDirs[$this->getFilename()]) && $this->isDir()) {
59+
return false;
60+
}
61+
62+
if ($this->excludedPattern) {
63+
$path = $this->isDir() ? $this->current()->getRelativePathname() : $this->current()->getRelativePath();
64+
$path = str_replace('\\', '/', $path);
65+
66+
return !preg_match($this->excludedPattern, $path);
5467
}
5568

5669
return true;
5770
}
5871

5972
public function hasChildren()
6073
{
61-
return $this->isRecursive && $this->getInnerIterator()->hasChildren();
74+
return $this->isRecursive && $this->iterator->hasChildren();
6275
}
6376

6477
public function getChildren()
6578
{
66-
$children = new self($this->getInnerIterator()->getChildren(), array());
67-
$children->patterns = $this->patterns;
79+
$children = new self($this->iterator->getChildren(), array());
80+
$children->excludedDirs = $this->excludedDirs;
81+
$children->excludedPattern = $this->excludedPattern;
6882

6983
return $children;
7084
}

‎src/Symfony/Component/Finder/Iterator/RecursiveDirectoryIterator.php

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@ class RecursiveDirectoryIterator extends \RecursiveDirectoryIterator
3131
*/
3232
private $rewindable;
3333

34+
// these 3 properties take part of the performance optimization to avoid redoing the same work in all iterations
35+
private $rootPath;
36+
private $subPath;
37+
private $directorySeparator = '/';
38+
3439
/**
3540
* Constructor.
3641
*
@@ -48,6 +53,10 @@ public function __construct($path, $flags, $ignoreUnreadableDirs = false)
4853

4954
parent::__construct($path, $flags);
5055
$this->ignoreUnreadableDirs = $ignoreUnreadableDirs;
56+
$this->rootPath = (string) $path;
57+
if ('/' !== DIRECTORY_SEPARATOR && !($flags & self::UNIX_PATHS)) {
58+
$this->directorySeparator = DIRECTORY_SEPARATOR;
59+
}
5160
}
5261

5362
/**
@@ -57,7 +66,17 @@ public function __construct($path, $flags, $ignoreUnreadableDirs = false)
5766
*/
5867
public function current()
5968
{
60-
return new SplFileInfo(parent::current()->getPathname(), $this->getSubPath(), $this->getSubPathname());
69+
// the logic here avoids redoing the same work in all iterations
70+
71+
if (null === $subPathname = $this->subPath) {
72+
$subPathname = $this->subPath = (string) $this->getSubPath();
73+
}
74+
if ('' !== $subPathname) {
75+
$subPathname .= $this->directorySeparator;
76+
}
77+
$subPathname .= $this->getFilename();
78+
79+
return new SplFileInfo($this->rootPath.$this->directorySeparator.$subPathname, $this->subPath, $subPathname);
6180
}
6281

6382
/**
@@ -73,6 +92,10 @@ public function getChildren()
7392
if ($children instanceof self) {
7493
// parent method will call the constructor with default arguments, so unreadable dirs won't be ignored anymore
7594
$children->ignoreUnreadableDirs = $this->ignoreUnreadableDirs;
95+
96+
// performance optimization to avoid redoing the same work in all children
97+
$children->rewindable = &$this->rewindable;
98+
$children->rootPath = $this->rootPath;
7699
}
77100

78101
return $children;

0 commit comments

Comments
 (0)
0