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
This commit is contained in:
commit
4e7644c597
@ -18,8 +18,10 @@ namespace Symfony\Component\Finder\Iterator;
|
|||||||
*/
|
*/
|
||||||
class ExcludeDirectoryFilterIterator extends FilterIterator implements \RecursiveIterator
|
class ExcludeDirectoryFilterIterator extends FilterIterator implements \RecursiveIterator
|
||||||
{
|
{
|
||||||
|
private $iterator;
|
||||||
private $isRecursive;
|
private $isRecursive;
|
||||||
private $patterns;
|
private $excludedDirs = array();
|
||||||
|
private $excludedPattern;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Constructor.
|
* Constructor.
|
||||||
@ -29,10 +31,18 @@ class ExcludeDirectoryFilterIterator extends FilterIterator implements \Recursiv
|
|||||||
*/
|
*/
|
||||||
public function __construct(\Iterator $iterator, array $directories)
|
public function __construct(\Iterator $iterator, array $directories)
|
||||||
{
|
{
|
||||||
|
$this->iterator = $iterator;
|
||||||
$this->isRecursive = $iterator instanceof \RecursiveIterator;
|
$this->isRecursive = $iterator instanceof \RecursiveIterator;
|
||||||
$this->patterns = array();
|
$patterns = array();
|
||||||
foreach ($directories as $directory) {
|
foreach ($directories as $directory) {
|
||||||
$this->patterns[] = '#(^|/)'.preg_quote($directory, '#').'(/|$)#';
|
if (!$this->isRecursive || false !== strpos($directory, '/')) {
|
||||||
|
$patterns[] = preg_quote($directory, '#');
|
||||||
|
} else {
|
||||||
|
$this->excludedDirs[$directory] = true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if ($patterns) {
|
||||||
|
$this->excludedPattern = '#(?:^|/)(?:'.implode('|', $patterns).')(?:/|$)#';
|
||||||
}
|
}
|
||||||
|
|
||||||
parent::__construct($iterator);
|
parent::__construct($iterator);
|
||||||
@ -45,12 +55,15 @@ class ExcludeDirectoryFilterIterator extends FilterIterator implements \Recursiv
|
|||||||
*/
|
*/
|
||||||
public function accept()
|
public function accept()
|
||||||
{
|
{
|
||||||
$path = $this->isDir() ? $this->current()->getRelativePathname() : $this->current()->getRelativePath();
|
if ($this->isRecursive && isset($this->excludedDirs[$this->getFilename()]) && $this->isDir()) {
|
||||||
$path = str_replace('\\', '/', $path);
|
|
||||||
foreach ($this->patterns as $pattern) {
|
|
||||||
if (preg_match($pattern, $path)) {
|
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if ($this->excludedPattern) {
|
||||||
|
$path = $this->isDir() ? $this->current()->getRelativePathname() : $this->current()->getRelativePath();
|
||||||
|
$path = str_replace('\\', '/', $path);
|
||||||
|
|
||||||
|
return !preg_match($this->excludedPattern, $path);
|
||||||
}
|
}
|
||||||
|
|
||||||
return true;
|
return true;
|
||||||
@ -58,13 +71,14 @@ class ExcludeDirectoryFilterIterator extends FilterIterator implements \Recursiv
|
|||||||
|
|
||||||
public function hasChildren()
|
public function hasChildren()
|
||||||
{
|
{
|
||||||
return $this->isRecursive && $this->getInnerIterator()->hasChildren();
|
return $this->isRecursive && $this->iterator->hasChildren();
|
||||||
}
|
}
|
||||||
|
|
||||||
public function getChildren()
|
public function getChildren()
|
||||||
{
|
{
|
||||||
$children = new self($this->getInnerIterator()->getChildren(), array());
|
$children = new self($this->iterator->getChildren(), array());
|
||||||
$children->patterns = $this->patterns;
|
$children->excludedDirs = $this->excludedDirs;
|
||||||
|
$children->excludedPattern = $this->excludedPattern;
|
||||||
|
|
||||||
return $children;
|
return $children;
|
||||||
}
|
}
|
||||||
|
@ -31,6 +31,11 @@ class RecursiveDirectoryIterator extends \RecursiveDirectoryIterator
|
|||||||
*/
|
*/
|
||||||
private $rewindable;
|
private $rewindable;
|
||||||
|
|
||||||
|
// these 3 properties take part of the performance optimization to avoid redoing the same work in all iterations
|
||||||
|
private $rootPath;
|
||||||
|
private $subPath;
|
||||||
|
private $directorySeparator = '/';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Constructor.
|
* Constructor.
|
||||||
*
|
*
|
||||||
@ -48,6 +53,10 @@ class RecursiveDirectoryIterator extends \RecursiveDirectoryIterator
|
|||||||
|
|
||||||
parent::__construct($path, $flags);
|
parent::__construct($path, $flags);
|
||||||
$this->ignoreUnreadableDirs = $ignoreUnreadableDirs;
|
$this->ignoreUnreadableDirs = $ignoreUnreadableDirs;
|
||||||
|
$this->rootPath = (string) $path;
|
||||||
|
if ('/' !== DIRECTORY_SEPARATOR && !($flags & self::UNIX_PATHS)) {
|
||||||
|
$this->directorySeparator = DIRECTORY_SEPARATOR;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -57,7 +66,17 @@ class RecursiveDirectoryIterator extends \RecursiveDirectoryIterator
|
|||||||
*/
|
*/
|
||||||
public function current()
|
public function current()
|
||||||
{
|
{
|
||||||
return new SplFileInfo(parent::current()->getPathname(), $this->getSubPath(), $this->getSubPathname());
|
// the logic here avoids redoing the same work in all iterations
|
||||||
|
|
||||||
|
if (null === $subPathname = $this->subPath) {
|
||||||
|
$subPathname = $this->subPath = (string) $this->getSubPath();
|
||||||
|
}
|
||||||
|
if ('' !== $subPathname) {
|
||||||
|
$subPathname .= $this->directorySeparator;
|
||||||
|
}
|
||||||
|
$subPathname .= $this->getFilename();
|
||||||
|
|
||||||
|
return new SplFileInfo($this->rootPath.$this->directorySeparator.$subPathname, $this->subPath, $subPathname);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -73,6 +92,10 @@ class RecursiveDirectoryIterator extends \RecursiveDirectoryIterator
|
|||||||
if ($children instanceof self) {
|
if ($children instanceof self) {
|
||||||
// parent method will call the constructor with default arguments, so unreadable dirs won't be ignored anymore
|
// parent method will call the constructor with default arguments, so unreadable dirs won't be ignored anymore
|
||||||
$children->ignoreUnreadableDirs = $this->ignoreUnreadableDirs;
|
$children->ignoreUnreadableDirs = $this->ignoreUnreadableDirs;
|
||||||
|
|
||||||
|
// performance optimization to avoid redoing the same work in all children
|
||||||
|
$children->rewindable = &$this->rewindable;
|
||||||
|
$children->rootPath = $this->rootPath;
|
||||||
}
|
}
|
||||||
|
|
||||||
return $children;
|
return $children;
|
||||||
|
Reference in New Issue
Block a user