Merge lp:~catch-drupal/pressflow/pressflow_implements into lp:pressflow

[Nathaniel Catchpole]

Backport of Drupal 7 module_implements() caching. Adds one extra change to path.inc since the return value of $modules is now an associative array instead of numeric, otherwise identical to the core patch.

On a site with 150 modules instead, this reduced page execution time of module_implements() from around 4% of the request to less than 1%.

[lp:~catch-drupal/pressflow/pressflow_implements]

100. By Nathaniel <catch@catch-toshiba> on 2011-07-25

Merging in trunk.

[David Strauss (davidstrauss)]

Pressflow 6 is now maintained on GitHub, so I'm marking this merge as rejected.

Unmerged revisions

100. By Nathaniel <catch@catch-toshiba> on 2011-07-25

Merging in trunk.

99. By Nathaniel Catchpole <catch@catch-laptop> on 2011-02-15

Merge in trunk.

98. By Nathaniel Catchpole <catch@catch-laptop> on 2010-11-15

 is now an associative array, so use reset instead of a hard-coded index.

97. By Nathaniel Catchpole <catch@catch-laptop> on 2010-11-15

#557542: module_implements() caching.

=== modified file 'includes/common.inc'
--- includes/common.inc	2011-06-01 07:13:05 +0000
+++ includes/common.inc	2011-07-25 06:23:28 +0000
@@ -1619,6 +1619,8 @@
   if ($cache != CACHE_DISABLED && $cache != CACHE_EXTERNAL) {
     page_set_cache();
   }
+  // Update the hook implementation cache.
+  module_implements('', FALSE, FALSE, TRUE);
 
   module_invoke_all('exit');
 }

=== modified file 'includes/module.inc'
--- includes/module.inc	2011-05-26 02:56:05 +0000
+++ includes/module.inc	2011-07-25 06:23:28 +0000
@@ -405,27 +405,78 @@
  * @param $sort
  *   By default, modules are ordered by weight and filename, settings this option
  *   to TRUE, module list will be ordered by module name.
- * @param $refresh
+ * @param $reset
  *   For internal use only: Whether to force the stored list of hook
  *   implementations to be regenerated (such as after enabling a new module,
  *   before processing hook_enable).
+ * @param $write_cache
+ *   For internal use only: Update the persistent cache of hook implementations.
  * @return
  *   An array with the names of the modules which are implementing this hook.
  */
-function module_implements($hook, $sort = FALSE, $refresh = FALSE) {
+function module_implements($hook, $sort = FALSE, $reset = FALSE, $write_cache = FALSE) {
   static $implementations;
 
-  if ($refresh) {
+  // We maintain a persistent cache of hook implementations in addition to the
+  // static cache to avoid looping through every module and every hook on each
+  // request. Benchmarks show that the benefit of this caching outweighs the
+  // additional database hit even when using the default database caching
+  // backend and only a small number of modules are enabled. The cost of the
+  // cache_get() is more or less constant and reduced further when non-database
+  // caching backends are used, so there will be more significant gains when a
+  // large number of modules are installed or hooks invoked, since this can
+  // quickly lead to module_hook() being called several thousand times
+  // per request.
+  if ($reset) {
     $implementations = array();
-    return;
+    cache_set('module_implements', array());
+    return;
+  }
+
+  if ($write_cache) {
+    // Check whether we should write the cache. We do not want to cache hooks
+    // which are only invoked on HTTP POST requests since these do not need to
+    // be optimized as tightly, and not doing so keeps the cache entry smaller.
+    if (isset($implementations['#write_cache']) && ($_SERVER['REQUEST_METHOD'] == 'GET' || $_SERVER['REQUEST_METHOD'] == 'HEAD')) {
+      unset($implementations['#write_cache']);
+      cache_set('module_implements', $implementations);
+    }
+    return;
+  }
+
+  // Fetch implementations from cache.
+  if (empty($implementations)) {
+    $cache = cache_get('module_implements');
+    if (!$cache) {
+      $implementations = array();
+    }
+    else {
+      $implementations = $cache->data;
+    }
   }
 
   if (!isset($implementations[$hook])) {
+    // The hook is not cached, so ensure that whether or not it has
+    // implementations, that the cache is updated at the end of the request.
+    $implementations['#write_cache'] = TRUE;
     $implementations[$hook] = array();
     $list = module_list(FALSE, TRUE, $sort);
     foreach ($list as $module) {
       if (module_hook($module, $hook)) {
-        $implementations[$hook][] = $module;
+        $implementations[$hook][$module] = $module;
+      }
+    }
+  }
+  else {
+    foreach ($implementations[$hook] as $module) {
+      // It is possible that a module removed a hook implementation without the
+      // implementations cache being rebuilt yet, so we check module_hook() on
+      // each request to avoid undefined function errors.
+      if (!module_hook($module, $hook)) {
+        // Clear out the stale implementation from the cache and force a cache
+        // refresh to forget about no longer existing hook implementations.
+        unset($implementations[$hook][$module]);
+        $implementations['#write_cache'] = TRUE;
       }
     }
   }

=== modified file 'includes/path.inc'
--- includes/path.inc	2011-05-26 02:58:29 +0000
+++ includes/path.inc	2011-07-25 06:23:28 +0000
@@ -50,7 +50,7 @@
   if (!isset($hook_module)) {
     $modules = module_implements('lookup_path');
     if (count($modules) > 0) {
-      $hook_module = $modules[0];
+      $hook_module = reset($modules);
     } else {
       $hook_module = FALSE;
     }

=== modified file 'install.php'
--- install.php	2011-05-26 02:58:29 +0000
+++ install.php	2011-07-25 06:23:28 +0000
@@ -47,6 +47,18 @@
   drupal_load('module', 'system');
   drupal_load('module', 'filter');
 
+  // Load the cache infrastructure using a "fake" cache implementation that
+  // does not attempt to write to the database. We need this during the initial
+  // part of the installer because the database is not available yet. We
+  // continue to use it even when the database does become available, in order
+  // to preserve consistency between interactive and command-line installations
+  // (the latter complete in one page request and therefore are forced to
+  // continue using the cache implementation they started with) and also
+  // because any data put in the cache during the installer is inherently
+  // suspect, due to the fact that Drupal is not fully set up yet.
+  require_once './includes/cache-install.inc';
+  $conf['cache_inc'] = './includes/cache-install.inc';
+
   // Install profile chosen, set the global immediately.
   // This needs to be done before the theme cache gets 
   // initialized in drupal_maintenance_theme().
@@ -61,12 +73,6 @@
   $verify = install_verify_settings();
 
   if ($verify) {
-    // Since we have a database connection, we use the normal cache system.
-    // This is important, as the installer calls into the Drupal system for
-    // the clean URL checks, so we should maintain the cache properly.
-    require_once './includes/cache.inc';
-    $conf['cache_inc'] = './includes/cache.inc';
-
     // Establish a connection to the database.
     require_once './includes/database.inc';
     db_set_active();
@@ -78,13 +84,6 @@
     }
   }
   else {
-    // Since no persistent storage is available yet, and functions that check
-    // for cached data will fail, we temporarily replace the normal cache
-    // system with a stubbed-out version that short-circuits the actual
-    // caching process and avoids any errors.
-    require_once './includes/cache-install.inc';
-    $conf['cache_inc'] = './includes/cache-install.inc';
-
     $task = NULL;
   }
 
@@ -815,17 +814,14 @@
 
   // The end of the install process. Remember profile used.
   if ($task == 'done') {
-    // Rebuild menu to get content type links registered by the profile,
-    // and possibly any other menu items created through the tasks.
-    menu_rebuild();
+    // Flush all caches to ensure that any full bootstraps during the installer
+    // do not leave stale cached data, and that any content types or other items
+    // registered by the install profile are registered correctly.
+    drupal_flush_all_caches();
 
     // Register actions declared by any modules.
     actions_synchronize();
 
-    // Randomize query-strings on css/js files, to hide the fact that
-    // this is a new install, not upgraded yet.
-    _drupal_flush_css_js();
-
     variable_set('install_profile', $profile);
   }
 

=== modified file 'update.php'
--- update.php	2011-05-26 02:58:29 +0000
+++ update.php	2011-07-25 06:23:28 +0000
@@ -594,7 +594,7 @@
 $op = isset($_REQUEST['op']) ? $_REQUEST['op'] : '';
 if (empty($op)) {
   // Minimum load of components.
-  drupal_bootstrap(DRUPAL_BOOTSTRAP_CONFIGURATION);
+  drupal_bootstrap(DRUPAL_BOOTSTRAP_DATABASE);
 
   require_once './includes/install.inc';
   require_once './includes/file.inc';
@@ -605,6 +605,8 @@
   $module_list['system']['filename'] = 'modules/system/system.module';
   $module_list['filter']['filename'] = 'modules/filter/filter.module';
   module_list(TRUE, FALSE, FALSE, $module_list);
+  module_implements('', FALSE, TRUE);
+
   drupal_load('module', 'system');
   drupal_load('module', 'filter');