Hi there,
I am a bit concerned about the wysiwyg_get_directories() function located around line 970 of wysiwyg.module. The technical reasons to have the function actually call module_implements() seem clear (to allow other modules to extend the wysiwyg module easily), I'm worried about the efficiency of the function where no cache system stores the static $implementations array at the beginning of module_implements() (see http://api.drupal.org/api/drupal/includes--module.inc/function/module_im...).
I'm imagining most sites using wysiwyg will have between 40 and 120 modules, and in such case the system would have to go through, for each page load, the 40-120 modules just to see that, in most cases (easily 99%), none of them actually extends wysiwyg...
Wouldn't it be a good idea to add a setting to tell wysiwyg not to call module_implements() (which executes module_hook() which executes a concatenation for each module - so very resource hungry)?
Just asking...
Comments
Comment #1
ywarnier commentedForgot to mention, as a suggestion, the variable setting could be in the wysiwyg profile page in an additional "Efficiency" block and be selected by default and say "I don't need to check for modules that extend WYSIWYG because no module extend it on this site (this will improve my site speed but will prevent me to fully extend WYSIWYG)". Something like that.
Comment #2
twodHow much would the performance improvement be here?
module_implements() calls module_list() which already has had its cache populated during bootstrap, so this essentially boils down to function_exists() being called once for each module.
I don't actually know the performance penalty for that so I'd love to see some benchmarks.
Comment #3
sunThere's no measurable performance impact in module_implements(), even with hundreds of modules installed. It's used everywhere throughout Drupal core and contrib; it's the base for module_invoke_all() and most other module system functions.
@ywarnier: Before posting such issues, please confirm your opinion/beliefs by doing performance benchmarks. If you created similar issues elsewhere, then close them accordingly, please.
Comment #4
ywarnier commentedHi there, sorry for the delay answering. I'll eventually get back on this with a proper benchmark. Thanks.