Merge lp:~glennpratt/pressflow/module_implements-cache into lp:pressflow

[Glenn Pratt]

Apply backport of Drupal 7 module_implements cache.

http://drupal.org/node/557542#comment-4038288

[Nathaniel Catchpole (catch-drupal)]

There's another merge proposal for this at https://code.launchpad.net/~catch-drupal/pressflow/pressflow_implements

That also (as of today, I forgot to commit it in November) has an additional fix to Pressflow's path.inc - without that the path_alias_cache module (and path aliases in general) will be non-functional when enabled).

review: Disapprove

[Glenn Pratt (glennpratt)]

Thanks for looking, I will compare your merge and try to migrate to it.

Unmerged revisions

98. By Glenn Pratt on 2011-02-02

Add module_implement cache backport from Drupal 7. See http://drupal.org/node/557542

=== modified file 'includes/common.inc'
--- includes/common.inc	2010-12-16 19:39:23 +0000
+++ includes/common.inc	2011-02-02 17:49:08 +0000
@@ -1622,6 +1622,9 @@
   }
 
   module_invoke_all('exit');
+
+  // Update the hook implementation cache.
+  module_implements('', FALSE, FALSE, TRUE);
 }
 
 /**

=== modified file 'includes/module.inc'
--- includes/module.inc	2010-12-16 19:28:18 +0000
+++ includes/module.inc	2011-02-02 17:49:08 +0000
@@ -406,27 +406,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 'install.php'
--- install.php	2010-12-16 19:39:23 +0000
+++ install.php	2011-02-02 17:49:08 +0000
@@ -48,6 +48,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().
@@ -62,12 +74,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();
@@ -79,13 +85,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;
   }
 
@@ -816,17 +815,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	2010-12-16 19:39:23 +0000
+++ update.php	2011-02-02 17:49:08 +0000
@@ -595,7 +595,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';
@@ -606,6 +606,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');