Closed (fixed)
Project:
Zen
Version:
6.x-2.x-dev
Component:
PHP/JS code
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
5 Nov 2009 at 08:57 UTC
Updated:
4 Sep 2013 at 20:21 UTC
Jump to comment: Most recent file
drupal_get_filename() is broken; see #341140: drupal_get_filename() when database is down, does not deal with phptemplate themes.
When that is fixed in Drupal 6, replace _zen_path() with drupal_get_path('theme', 'zen').
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | zen.6-x-2-x._zen_path-performance.624058-6.patch | 853 bytes | donquixote |
Comments
Comment #1
johnalbinComment #2
akalata commentedComment #3
donquixote commenteddrupal_system_listing() is expensive.
I don't understand the exact problem with
drupal_get_path('theme', 'zen'), but following the title of #341140: drupal_get_filename() when database is down, does not deal with phptemplate themes, it sounds like this only applies if the database is down? Also, what exactly happens if you call drupal_get_path() instead of _zen_path() ?My proposal:
We check if the database is there, or if whatever condition applies that allows us to use drupal_get_path().
We could also check if the returned path contains the zen.info file.
Only if this fails, we call drupal_system_listing().
I will create a patch, if someone can tell me what the exact problem is we need to work around.
--------------
For reference, here the current code of _zen_path(): (in 6.x-2.1)
Comment #4
donquixote commentedHere is my proposed replacement of _zen_path().
I could make this a patch, but for now I just want some feedback..
Comment #5
barrapontolooks good. but if you want actual feedback, from people testing the code and benchmarking, you got to provide a path. but i believe it makes lots of sense.
Comment #6
donquixote commentedHere we go.
Comment #7
j0rd commentedBefore this patch, _zen_path (on my VM, slow disk access) in XHProf took the most amount of time of any function call to execute. This was due to around 3000-6000 calls to is_dir. With this patch, is_dir is called twice upon them being primed in the database.
Before patch, load time for primed homepage ~2.5 seconds. Calls to is_dir 1,600 taking 600 ms.
After patch, load time for primed homepage ~1.5 seconds. Calls to is_dir 12, taking 24 ms.
catch's core patch does roughly the same thing as this patch. http://drupal.org/node/341140#comment-4300406
I can't imagine Drupal core changing anything anytime soon, so while we wait, I'd recommend we implement this.
Comment #8
j0rd commentedNot sure why tag changed, resetting. Also changing status to reviewed & tested.
Comment #9
johnalbinI don't know why the $path was renamed to $_path, so I changed it back. Otherwise it's fine. :-)
Comment #10
donquixote commentedStupid habit I once had, naming all static variables with a leading underscore.