Problem/Motivation
drupal_system_listing() is called by four separate branches of logic during Drupal 7 installation, but provides no static caching for the results of its filesystem scan. Profiling with cachegrind shows this is a good candidate for improving Drupal installation performance (see #686196-107: Meta issue: Install failures on various hosting environments), which may alleviate the fact Drupal installation times out on some hosts.
Proposed resolution
Add static caching to drupal_system_listing().
Remaining tasks
- Review patch
- Benchmark patch
- Analyze memory use of related static caches (see #733308-13: Add static caching for drupal_system_listing()).
User interface changes
None.
API changes
None.
Original report by @catch
See http://drupal.org/node/686196#comment-2668760 and downwards for why. I doubt we have time to be doing heavy refactoring of the installer, so we should at least avoid the duplicate work, if not the duplicate function calls.
Comments
Comment #1
Crell commentedSubscribing.
Comment #2
carlos8f commentedme too
Comment #3
das-peter commentedWe should work out something asap. here.
Adding a simple static cache to
file_scan_directory()speeds up a 404 page load by 50% on my test environment.Attached patch contains the code I used to test this.
The question is, if there are any functions / constructs which use
file_scan_directory()ordrupal_system_listing()to detect/fetch changes that are made within the same request.Anyway I think we could add an additional parameter to reset the cache to provide an easy way to get a fresh listing.
Comment #4
das-peter commentedAttached patch adds static caching also to
drupal_system_listing().Now there's also the
$resetparameter and I tried to figure out where I've to force a fresh list - mostly on install / update actions.This patch doesn't claim to be complete - but it's hopefully a beginning. :)
Comment #6
das-peter commentedFixes related to the failed tests.
Comment #8
das-peter commented*grml* Sometimes I'm feeling stupid. Hope the tests pass now.
Comment #9
moshe weitzman commentedIMO this is off the table for D7. Lets refactor for D8.
Anyway, what is scanning directories during a 404 page?
Comment #10
das-peter commented@moshe I don't know what triggered the rescan on 404. Atm. I can't reproduce it :| But I think it was related to this issue #1081266: Avoid re-scanning module directory when a filename or a module is missing
Comment #11
catchMoving to 8.x, tagging for backport just in case, changing to a task since since it's not a show-stopping performance issue, but still major since it can cause timeouts and similar on some setups.
Comment #12
catch.
Comment #13
mfbAs I mentioned in #1081266: Avoid re-scanning module directory when a filename or a module is missing, drupal_get_filename() already has a static cache of $files found via drupal_system_listing(). So it'd be a good idea to analyze memory usage of what seems like redundant static caches.
Comment #14
catchPer discussion on #1081266: Avoid re-scanning module directory when a filename or a module is missing I'm marking this as duplicate.
Comment #15
j0rd commentedSame problem exists in D6 when clearing the caches. is_dir is called on my install roughly 10,000 times 90% of them being from drupal_system_listing(). This adds roughly 3 seconds to page load on my VM.
Comment #15.0
j0rd commentedAdded issue summary.
Comment #16
iainp999 commentedEDIT: apologies, just saw the comment re: duplicate :)
Sorry to bump this, but we found this to be an issue when profiling of feature revert on a D7 site that we're working on.
I know this is a D8 issue with potential to backport, but any reason why the issue was not progressed?
Thanks in advance.