In includes/theme.inc/drupal_theme_initialize(), finding the ancestors is done that way, accessing arrays and objects many times:
// Find all our ancestor themes and put them in an array.
$base_theme = array();
$ancestor = $theme;
while ($ancestor && isset($themes[$ancestor]->base_theme)) {
$ancestor = $themes[$ancestor]->base_theme;
$base_theme[] = $themes[$ancestor];
}
I think it can optimized (and made clearer/cleaner) thus:
// Find all our ancestor themes and put them in an array.
$base_theme = array();
$ancestor = $themes[$theme]->base_theme;
while ($ancestor) {
$base_theme[] = $themes[$ancestor];
$ancestor = $themes[$ancestor]->base_theme;
}
I tested it (see attached file):
- from 0 to 100 ancestors
- repeating the while from 1 to 1 million times in a row.
It constantly gave an improvement of around... 55 % !
Note: this patch also solves issue #832624 (http://drupal.org/node/832624)
Comments
Comment #1
LaurentAjdnik commentedComment #2
LaurentAjdnik commentedComment #4
twodThis patch assumes all themes have base themes (or at least the base_theme property).
There are several problems with the benchmarking code in the original post. I tried to run it on my AMD Phenom II X4 system with 16GB RAM and it failed miserably. It ate all RAM in a matter of seconds and was killled by the system after it was out of swap space as well, even when only using 10 base themes and 10 loops.
isset()is needed around$themes[$theme]->base_themein the suggested method or it throws a warning for each theme.I fixed the points above and added another test with the isset() calls.
I also added another test identical to the first to see if there's an error margin depending on when the test is run.
Then I ran it a couple of times and always got results similar to this.
The suggested method is slightly faster, but throws the warnings.
With the added isset() calls in the "Fixed method", there is no gain compared to the original code.
Test code: