-
Notifications
You must be signed in to change notification settings - Fork 9.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve directory tree read performance #30629
Closed
Closed
Changes from 4 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
ae2c269
Improve directory tree read performance
Nazar65 dbbe8f5
Improve code style
Nazar65 bc26500
Code style fixes
Nazar65 f668723
Static test fixes
Nazar65 636b0ca
Use Magento framework Glob, codereview suggestion
Nazar65 ddd3d8a
Cover chnages with integration test
Nazar65 8f8d3ff
Static test fixes
Nazar65 bfbf89e
Fix MediaGallery uploader plugin to cover only media path
Nazar65 557a240
Static test fixes
Nazar65 86ed340
Fix path's issue
Nazar65 743e44c
Split second issue
Nazar65 5b61221
Skip recursice reading in excluded folders
Nazar65 a8baf00
End loop to avoid recursive reading
Nazar65 1b175de
magento/magento2#30629: Code review adjustments
sivaschenko 5d09b0f
magento/magento2#30629: Optimized tree creation and improved integrat…
sivaschenko 1b7743a
magento/magento2#30629: Added missing strict return type
sivaschenko f81f0f7
magento/magento2#30629: Removed unnecessary dependencies
sivaschenko 9d80b97
Merge branch '2.4-develop' of github.com:magento/magento2 into direct…
sivaschenko 979a5bf
magento/magento2#30629: Updated integration test to handle class name…
sivaschenko 8c96154
Merge branch '2.4-develop' into directory-tree-performance
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,8 +10,9 @@ | |
use Magento\Framework\App\Filesystem\DirectoryList; | ||
use Magento\Framework\Exception\ValidatorException; | ||
use Magento\Framework\Filesystem; | ||
use Magento\Framework\Filesystem\Directory\Read; | ||
use Magento\MediaGalleryApi\Api\IsPathExcludedInterface; | ||
use Magento\Framework\Filesystem\Driver\File; | ||
use Magento\Framework\Filesystem\Io\File as FileIo; | ||
|
||
/** | ||
* Build media gallery folder tree structure by path | ||
|
@@ -28,15 +29,31 @@ class GetFolderTree | |
*/ | ||
private $isPathExcluded; | ||
|
||
/** | ||
* @var File | ||
*/ | ||
private $driver; | ||
|
||
/** | ||
* @var FileIo | ||
*/ | ||
private $file; | ||
|
||
/** | ||
* @param Filesystem $filesystem | ||
* @param File $driver | ||
* @param FileIo $file | ||
* @param IsPathExcludedInterface $isPathExcluded | ||
*/ | ||
public function __construct( | ||
Filesystem $filesystem, | ||
File $driver, | ||
FileIo $file, | ||
IsPathExcludedInterface $isPathExcluded | ||
) { | ||
$this->filesystem = $filesystem; | ||
$this->driver = $driver; | ||
$this->file = $file; | ||
$this->isPathExcluded = $isPathExcluded; | ||
} | ||
|
||
|
@@ -74,15 +91,13 @@ private function getDirectories(): array | |
{ | ||
$directories = []; | ||
|
||
/** @var Read $directory */ | ||
$directory = $this->filesystem->getDirectoryRead(DirectoryList::MEDIA); | ||
$mediaPath = $this->filesystem->getDirectoryRead(DirectoryList::MEDIA)->getAbsolutePath(); | ||
$directoryList = $this->recursiveRead($mediaPath . '*', GLOB_BRACE | GLOB_ONLYDIR); | ||
|
||
if (!$directory->isDirectory()) { | ||
return $directories; | ||
} | ||
foreach ($directoryList as $path) { | ||
$path = str_replace($mediaPath, '', $path); | ||
|
||
foreach ($directory->readRecursively() as $path) { | ||
if (!$directory->isDirectory($path) || $this->isPathExcluded->execute($path)) { | ||
if ($this->isPathExcluded->execute($path)) { | ||
continue; | ||
} | ||
|
||
|
@@ -99,6 +114,29 @@ private function getDirectories(): array | |
return $directories; | ||
} | ||
|
||
/** | ||
* Read only directories from file system | ||
* | ||
* @param string $pattern | ||
* @param int $flags | ||
*/ | ||
private function recursiveRead(string $pattern, int $flags = 0): array | ||
{ | ||
// phpcs:ignore Magento2.Functions.DiscouragedFunction | ||
$directories = glob($pattern, $flags); | ||
|
||
// phpcs:ignore Magento2.Functions.DiscouragedFunction | ||
foreach (glob($this->driver->getParentDirectory($pattern) . '/*', $flags) as $dir) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See previous comment |
||
//phpcs:ignore Magento2.Performance.ForeachArrayMerge | ||
$directories = array_merge( | ||
$directories, | ||
$this->recursiveRead($dir . '/' . $this->file->getPathInfo($pattern)['basename'], $flags) | ||
); | ||
} | ||
|
||
return $directories; | ||
} | ||
|
||
/** | ||
* Find parent directory | ||
* | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the \Magento\Framework\Filesystem\DriverInterface::search method be used instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@le0n4ik this would be great, but main idea is the flag to read directories only, and search method have no ability to set this flag, and it will search only for one level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but thanks, we can use Magento\Framework\Filesystem\Glob as well