Views Slideshow cause a huge drop of performance

confirming this.
have a slideshow with 6 slides and page load times are sometimes 15..30 seconds (only for slideshow)

verified that i don't suffer from the "module enabled but missing" issue mentioned in #1 / #1081266: Avoid re-scanning module directory when a filename or a module is missing

just wondered if this is about imagecache, in a way that images are resized too often when they in fact should already be in the cache.
but the files in my cache are weeks old so this seems not to be the case.

Just chipping in, I resolved this by including the files it needs by hand (checking which modules have the views_slideshow.inc) and wrapping it into a static check. Its a hack, but for now it worked.

@bastnic: cool you profiled it out!!

a bit of investigation shows this in fact comes from module_list().

here is a thread that, although being hard to read, might contain some ideas.
#222109: module_list() should be rewritten as 2 functions: module_list() and module_list_bootstrap()

Hmm I wonder if I should cache that call. Probably would help. Anybody want to take the time to wrap that in a cache?

Similar performance hit with 6.x. My coding skills are not good enough to provide a solution, but I am willing to test any patches if you want to.

I believe the proper way to fix this in 7.x is for the plugins to implement hook_hook_info(), which allows hooks placed in an include file to be auto-loaded by Drupal. Here's an example for views_slideshow_cycle:

function views_slideshow_cycle_hook_info() {
  $hooks = array(
    'views_slideshow_slideshow_info' => array(
      'group' => 'views_slideshow',
    ),
    'views_slideshow_option_definition' => array(
      'group' => 'views_slideshow',
    ),
    'views_slideshow_slideshow_type_form' => array(
      'group' => 'views_slideshow',
    ),
    'form_options_js' => array(
      'group' => 'views_slideshow',
    ),
    'views_slideshow_options_form_validate' => array(
      'group' => 'views_slideshow',
    ),
  );
  return $hooks;
}

Having this code in the plugin allows us to remove all instances of module_load_all_includes('views_slideshow.inc'), allows core caching take over, and eliminates hammering the filesystem on every page load.

I can confirm that the change in #8 indeed does get rid of all the is_file requests while the slideshow keeps functioning as is. Attaching two xhprof files for comparison. Its a basic site with only a handful of modules installed, but its clear that with the patch there is already 40 less is_file calls.

Rolling a patch as well, basically implementing #8.

Glad to hear it's working for you, snufkin. I believe we can actually remove all four references to module_load_all_includes once this hook is in place; re-rolling a patch for your consideration. We've been running this way since May with no issues.

Has anyone tested this with plugins other than cycle?

I should say, patching views_slideshow in this way allows it to work only with those plugins which are also patched in a similar way as views_slideshow_cycle here -- each plugin will need its own hook_hook_info implementation. For that reason I imagine this won't be part of 3.x...

I wouldn't feel comfortable having this in 3.x. It would be best if someone could wrap the original code in a cache call as suggested in #6. I haven't had a chance to look, but I believe views does the same thing as I do. It may be worth it to see how they accomplish this task.

The problem is that this is a call that discovers files and includes them. We could static cache this call, which would mean its only issued once during a pageload, but that is still one unnecessary scan of files. And we can't wrap this in permanent cache, since we do need the includes to happen. One hacky workaround would be to do a discovery (basically reimplementing the scan of files originating from module_load_all_includes() and cache that result.

The addition of hook_hook_info() does affect other plugins however, but are there so many modules that replace cycle? The only change those modules would have to do to stay compatible is to add the hook_hook_info() implementation. Having said that I understand the resistance towards adding it to 3.x, but please consider that this is a major performance issue and affects a ton of sites. Waiting with this relatively small improvement until 4.x comes out seems to be a bit of an overkill to me.

If that helps I'm happy to submit patches to those modules that aim to replace Views Slideshow: Cycle.

Do we have a roadmap for 4.x?

#11 you're right, i completely forgot there might be other calls to module_load_all_includes.

#14 You are right. I think doing a discovery and caching it is the best approach. The number of other modules is > 10 I believe. I would hate to have to make them do a change at this point.

Discovery and caching is what is already being done in core if you use the hooks given, so I'd hope this would be item #1 on a 4.x roadmap - make a clean break and let the main cache system do its thing.

Closed as duplicate #2395911: strace reporting requests for ".views_slideshow.inc" files for every single module

FWIW, I'm still applying patch #11 whenever I find this module still in use in production sites.

Although be nice to fix this, due to BC issues, wouldn't want to unless we did a 7.x-4.x series. That would be good in many ways but at this time not going to do so. If anyone has time/interest I would be happy to help with figuring out the codebase.