Fix performance issue in directory tree of MediaGallery #31749

thomas-kl1 · 2021-01-18T16:45:46Z

When the server has a bunch of files and directories, almost all the times the script dies because it can't iterate on time on all the inodes.

Description (*)

These changes allows to filter the directory on the fly, when the iterator is processed. Before that the filter was done after the iterator was loaded, so all the folder, even the excluded one, were loaded..

Fixed Issues (if relevant)

Fixes Magento 2.4.1 Media Gallery....not ready for prime time. Fails when you have large amounts of data (images) #30596

Related PRs

https://github.com/magento/magento2/pull/30629/files
The main difference is that here is we are using the directoryIterator object. The idea is to update the implement of ReadRecusively of the ReadInterface filesystem implementation to use the RecursiveCallbackFilterIterator. This callback is filtering the excluded directories in a single shot.

Manual testing scenarios (*)

Create a bunch of files and sub directories
Wait for the tree on the media gallery to render

m2-assistant · 2021-01-18T16:45:48Z

Hi @thomas-kl1. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE,
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests
Semantic Version Checker

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

thomas-kl1 · 2021-01-18T17:17:30Z

app/code/Magento/MediaGalleryUi/Model/Directories/GetDirectoryTree.php

+        $isExcluded = function (string $path) use ($directory): bool {
+            return $this->isPathExcluded->execute($directory->getRelativePath($path));
+        };
+        $flags = FilesystemIterator::SKIP_DOTS |
+            FilesystemIterator::UNIX_PATHS |
+            RecursiveDirectoryIterator::FOLLOW_SYMLINKS;
+
+        try {
+            $iterator = new RecursiveIteratorIterator(
+                new RecursiveCallbackFilterIterator(
+                    new RecursiveDirectoryIterator($directory->getAbsolutePath(), $flags),
+                    static function (
+                        SplFileInfo $file,
+                        $key,
+                        RecursiveIterator $iterator
+                    ) use ($isExcluded): bool {
+                        return $file->isDir() && $iterator->hasChildren() && !$isExcluded($file->getPathname());
+                    }
+                ),
+                RecursiveIteratorIterator::CHILD_FIRST
+            );
+            /** @var FilesystemIterator $file */
+            foreach ($iterator as $file) {
+                $path = $directory->getRelativePath($file->getPathname());
+                $directories[] = [
+                    'data' => $file->getBasename(),
+                    'attr' => ['id' => $file->getPathname()],
+                    'metadata' => [
+                        'path' => $path
+                    ],
+                    'path_array' => explode('/', $path)
+                ];


If the sample is validated I can implement a new FileDriver and override the readDirectoryRecursively to allow RecursiveCallbackFilterIterator

@thomas-kl1 Hi, i think that any performance fixes for this issue is like just temporary fixes, so there is only one way to fix this issue, is to update behavior to load folders on a first level path, and then render sub folders by ajax requests and get one by one as it was in old media gallery, and this is available for the jstree plugin, instead of reading the whole directory tree. If you have enough time to contribute you can implement this behavior as well, what's do you think ?

@Nazar65 that's true, anyway I've noticed that RecursiveIteratorIterator was used in Magento framework. However this shouldn't be used without a filter. I'm looking to add the possibility to give the filter as an argument to the method readRecursively.
Regarding the ajax, I agree it's the best solution to implement. I may have time this week to work on it.

@thomas-kl1 cool 👍

@Nazar65 I didn't had the time yet as it requires much more attention and development. I see how it was done in the old media gallery and I need to be sure it will be backward-compatible

@thomas-kl1 you don't need to change anything just provide a new component and then just change the jstree initialization, it's not a BIC changes providing new functionality

thomas-kl1 · 2021-01-18T17:19:45Z

@magento run all tests

thomas-kl1 · 2021-01-27T08:15:33Z

@magento run all tests

perfpcs · 2021-02-01T03:37:53Z

So I spent many many hours testing things this weekend and trying every possible combination of fixes. What I finally determined was the problem for me was having both my static files and media files on 2 different AWS Cloudfront Distributions. Here is what I determined this weekend:

Improve directory tree read performance #30629 this pull does actually work properly with large amounts of images, In my case over 2 million images. This is true only when using NO CDN and Magento Base URL's are set as URL's local to the server and not pointed to AWS Cloudfront CDN distributions. See settings:

Fix performance issue in directory tree of MediaGallery #31749 this pull does actually work properly with large amounts of images, In my case over 2 million. Again this works with settings pointing Static and Media to the local server via URL. Same as above.
Both Improve directory tree read performance #30629 and Fix performance issue in directory tree of MediaGallery #31749 DO NOT work when Magento Static Files and Media files are served from an AWS CDN. While the directory tree loads, the images do not always load. Clicking on folders changes nothing in views. Also the search bar does not appear and clicking on any folder shows no active filters to view or click and cancel. Also Adobe Stock Images does not work at all, you can not click and save images or previews. Clicking on anything in Adobe Stock does nothing.

4.#30629 DOES work fine with large amounts of images (in my case over 2 million images) when the Static URL is change back to local server. I WAS able to use the media Cloudfront distribution though. So, something about Magento files does not like the static files being served from a Cloudfront Distribution. Bottom line, with #30629 pull, Static URL pointed to local and media URL pointed to Cloudfront server, everything worked perfect. The search bar shows, clicking on folders adds them as the active filters that you can X out and close. Search terms work fine. Adobe Stock Images works perfectly and I can add previews and images with no problems. Basically, I can say everything works perfectly as expected with #30629 and just changing Static URL to the local server.

Fix performance issue in directory tree of MediaGallery #31749 for me does not work using any CDNs. I tried both Static and Media both, individual each, and nothing works with AWS Cloudfront. So Fix performance issue in directory tree of MediaGallery #31749 ONLY works with static and media URLs pointed to local URLs.

So, I hope this is helpful to those working on these files. I would love to have a fix to be able to use my Static file CDN on AWS. I am not sure what the issue is, I can only report what is happening. Thanks

thomas-kl1 · 2021-02-07T11:16:18Z

@magento run all tests

perfpcs · 2021-02-18T18:17:24Z

Just an update, after Amasty fixed some issues with their Extended Product Grid Extension that was conflicting with the new media gallery js files, I am finally able to have a fully working media gallery in the latest Magento 2.4.2 with the one file changed from #30629.
vendor/magento/module-media-gallery-ui/Model/Directories/GetDirectoryTree.php

With this one file change all now works correctly for us including using static files on CDN. The directory tree forms, the active filters work fine now and all aspects of Adobe Stock and adding images works properly.

The last time I tried this one #31749, it would not load media gallery for me unfortunately.

So hopefully #30629 is added to the next Magento release and I think all will be fine.
thanks for peoples help.

engcom-Charlie · 2022-12-05T10:58:03Z

Hi @thomas-kl1,

Thank you for your contribution!

As #35404 is open against the same related issue and it seems no activity on this draft from long time, closing this PR.

Thank you!

thomas-kl1 and others added 2 commits January 18, 2021 11:44

todo work

5a153cc

fix performance issue

d3a34de

magento-engcom-team added Component: MediaGalleryUi Release Line: 2.4 labels Jan 18, 2021

m2-community-project bot added the Progress: pending review label Jan 18, 2021

thomas-kl1 commented Jan 18, 2021

View reviewed changes

gabrieldagama added Priority: P1 Once P0 defects have been fixed, a defect having this priority is the next candidate for fixing. Severity: S1 Affects critical data or functionality and forces users to employ a workaround. labels Jan 19, 2021

Merge branch '2.4-develop' into patch-6

0141be1

thomas-kl1 mentioned this pull request Jan 27, 2021

Improve directory tree read performance #30629

Closed

4 tasks

m2-community-project bot assigned Nazar65 Jan 27, 2021

m2-community-project bot added Progress: review and removed Progress: pending review labels Jan 27, 2021

thomas-kl1 added 2 commits January 28, 2021 17:14

prepare getDirectories to allow maxDepth and base path

eb6b3f3

Merge branch 'patch-6' of github.com:thomas-kl1/magento2 into patch-6

46c9f6a

perfpcs mentioned this pull request Feb 1, 2021

Magento 2.4.1 Media Gallery....not ready for prime time. Fails when you have large amounts of data (images) #30596

Open

4 tasks

fix method signature

ec78bc2

thomas-kl1 force-pushed the patch-6 branch from 29bae03 to ec78bc2 Compare February 7, 2021 11:15

thomas-kl1 marked this pull request as draft February 22, 2021 10:02

engcom-Charlie self-requested a review December 5, 2022 10:54

engcom-Charlie closed this Dec 5, 2022

m2-community-project bot removed the Progress: review label Dec 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix performance issue in directory tree of MediaGallery #31749

Fix performance issue in directory tree of MediaGallery #31749

thomas-kl1 commented Jan 18, 2021 •

edited

Loading

m2-assistant bot commented Jan 18, 2021

thomas-kl1 Jan 18, 2021

Nazar65 Jan 27, 2021 •

edited

Loading

thomas-kl1 Jan 27, 2021 •

edited

Loading

Nazar65 Jan 27, 2021

thomas-kl1 Feb 9, 2021

Nazar65 Feb 9, 2021 •

edited

Loading

thomas-kl1 commented Jan 18, 2021

thomas-kl1 commented Jan 27, 2021

perfpcs commented Feb 1, 2021 •

edited

Loading

thomas-kl1 commented Feb 7, 2021

perfpcs commented Feb 18, 2021

engcom-Charlie commented Dec 5, 2022

Fix performance issue in directory tree of MediaGallery #31749

Fix performance issue in directory tree of MediaGallery #31749

Conversation

thomas-kl1 commented Jan 18, 2021 • edited Loading

Description (*)

Fixed Issues (if relevant)

Related PRs

Manual testing scenarios (*)

m2-assistant bot commented Jan 18, 2021

thomas-kl1 Jan 18, 2021

Choose a reason for hiding this comment

Nazar65 Jan 27, 2021 • edited Loading

Choose a reason for hiding this comment

thomas-kl1 Jan 27, 2021 • edited Loading

Choose a reason for hiding this comment

Nazar65 Jan 27, 2021

Choose a reason for hiding this comment

thomas-kl1 Feb 9, 2021

Choose a reason for hiding this comment

Nazar65 Feb 9, 2021 • edited Loading

Choose a reason for hiding this comment

thomas-kl1 commented Jan 18, 2021

thomas-kl1 commented Jan 27, 2021

perfpcs commented Feb 1, 2021 • edited Loading

thomas-kl1 commented Feb 7, 2021

perfpcs commented Feb 18, 2021

engcom-Charlie commented Dec 5, 2022

thomas-kl1 commented Jan 18, 2021 •

edited

Loading

Nazar65 Jan 27, 2021 •

edited

Loading

thomas-kl1 Jan 27, 2021 •

edited

Loading

Nazar65 Feb 9, 2021 •

edited

Loading

perfpcs commented Feb 1, 2021 •

edited

Loading