Remove unecessary module_rebuild_cache() call from _registry_rebuild()
Berdir - April 9, 2009 - 17:04
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | base system |
| Category: | task |
| Priority: | critical |
| Assigned: | Berdir |
| Status: | closed |
Description
Currently, _registry_rebuild calls module_rebuild_cache() just to get the list of available modules. This is slow because it executes a UPDATE-query for every enabled moduled and also parses all info files, even those of disabled modules.
I discussed this with chx (ok, the other way round, he explained me that we don't need it :)) and it seems we can replace it with just calling drupal_system_listing(), system_get_files_database() and drupal_parse_info_file() (only for enabled modules, in opposite to module_rebuild_cache())
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| drupal.registry_rebuild.patch | 1.11 KB | Idle | Unable to apply patch drupal.registry_rebuild.patch | View details |
#1
Improved inline comment.
#2
+1 - this would be a good performance boost!
I've applied and tried it out, and have had no problem rebuilding the code registry with this patch. Don't have benchmarks, though.
#3
admin/build/modules
Before:
Executed 668 queries in 226.55 milliseconds.
After:
Executed 607 queries in 234.22 milliseconds.
This is a query for every file in every module right? So presumably we could save several hundred queries when contrib modules are installed.
#4
@catch It saves atleast one UPDATE query for every module, not sure if there is more.
In your test, it is actually slower with my patch, even if there are less queries. I assume this has another reason that doesn't have anything to do with my patch.
#5
I think that's just because there's a lot of queries so potentially lots of variation.
I just hit the page a couple more times and got:
Executed 607 queries in 211.78 milliseconds.
So the patch isn't slowing anything down. If we need benchmarks for this I'll run them, but just looks like a no-brainer to me.
#6
Good catch, and looks good. Committed to CVS HEAD.
#7
Automatically closed -- issue fixed for 2 weeks with no activity.