From b14349889e9e51fb44934694030e95306918e3bd Mon Sep 17 00:00:00 2001 From: Angie Byron <webchick@24967.no-reply.drupal.org> Date: Fri, 20 Nov 2009 06:12:45 +0000 Subject: [PATCH] #619666 by effulgentsia, casey, and catch: Changed Introduce new pattern for drupal_static() in performance-critical functions. --- includes/bootstrap.inc | 79 ++++++++++++++++++++++++++++++++++++++++ includes/common.inc | 22 ++++++++--- includes/module.inc | 5 ++- includes/path.inc | 37 ++++++++++++++----- modules/user/user.module | 5 ++- 5 files changed, 131 insertions(+), 17 deletions(-) diff --git a/includes/bootstrap.inc b/includes/bootstrap.inc index 803212c14606..f0a5ec12a5d2 100644 --- a/includes/bootstrap.inc +++ b/includes/bootstrap.inc @@ -2086,6 +2086,83 @@ function registry_rebuild() { /** * Central static variable storage. * + * All functions requiring a static variable to persist or cache data within + * a single page request are encouraged to use this function unless it is + * absolutely certain that the static variable will not need to be reset during + * the page request. By centralizing static variable storage through this + * function, other functions can rely on a consistent API for resetting any + * other function's static variables. + * + * Example: + * @code + * function language_list($field = 'language') { + * $languages = &drupal_static(__FUNCTION__); + * if (!isset($languages)) { + * // If this function is being called for the first time after a reset, + * // query the database and execute any other code needed to retrieve + * // information about the supported languages. + * ... + * } + * if (!isset($languages[$field])) { + * // If this function is being called for the first time for a particular + * // index field, then execute code needed to index the information already + * // available in $languages by the desired field. + * ... + * } + * // Subsequent invocations of this function for a particular index field + * // skip the above two code blocks and quickly return the already indexed + * // information. + * return $languages[$field]; + * } + * function locale_translate_overview_screen() { + * // When building the content for the translations overview page, make + * // sure to get completely fresh information about the supported languages. + * drupal_static_reset('language_list'); + * ... + * } + * @endcode + * + * In a few cases, a function can have certainty that there is no legitimate + * use-case for resetting that function's static variable. This is rare, + * because when writing a function, it's hard to forecast all the situations in + * which it will be used. A guideline is that if a function's static variable + * does not depend on any information outside of the function that might change + * during a single page request, then it's ok to use the "static" keyword + * instead of the drupal_static() function. + * + * Example: + * @code + * function actions_do(...) { + * // $stack tracks the number of recursive calls. + * static $stack; + * $stack++; + * if ($stack > variable_get('actions_max_stack', 35)) { + * ... + * return; + * } + * ... + * $stack--; + * } + * @endcode + * + * In a few cases, a function needs a resettable static variable, but the + * function is called many times (100+) during a single page request, so + * every microsecond of execution time that can be removed from the function + * counts. These functions can use a more cumbersome, but faster variant of + * calling drupal_static(). For benchmarks and background on this variant, + * please see http://drupal.org/node/619666. + * + * Example: + * @code + * function user_access($string, $account = NULL) { + * // Use the advanced drupal_static() pattern, since this is called very often. + * static $drupal_static = array(); + * isset($drupal_static[__FUNCTION__]) || ($drupal_static[__FUNCTION__] = &drupal_static(__FUNCTION__)); + * $perm = &$drupal_static[__FUNCTION__]; + * ... + * } + * @endcode + * * @param $name * Globally unique name for the variable. For a function with only one static, * variable, the function name (e.g. via the PHP magic __FUNCTION__ constant) @@ -2102,6 +2179,8 @@ function registry_rebuild() { * * @return * Returns a variable by reference. + * + * @see drupal_static_reset() */ function &drupal_static($name, $default_value = NULL, $reset = FALSE) { static $data = array(), $default = array(); diff --git a/includes/common.inc b/includes/common.inc index 197124868e45..be8f2a974f1d 100644 --- a/includes/common.inc +++ b/includes/common.inc @@ -2275,7 +2275,11 @@ function format_interval($timestamp, $granularity = 2, $langcode = NULL) { * A translated date string in the requested format. */ function format_date($timestamp, $type = 'medium', $format = '', $timezone = NULL, $langcode = NULL) { - $timezones = &drupal_static(__FUNCTION__, array()); + // Use the advanced drupal_static() pattern, since this is called very often. + static $drupal_static = array(); + isset($drupal_static[__FUNCTION__]) || ($drupal_static[__FUNCTION__] = &drupal_static(__FUNCTION__)); + $timezones = &$drupal_static[__FUNCTION__]; + if (!isset($timezone)) { global $user; if (variable_get('configurable_timezones', 1) && $user->uid && $user->timezone) { @@ -2512,7 +2516,10 @@ function url($path = NULL, array $options = array()) { } global $base_url, $base_secure_url, $base_insecure_url; - $script = &drupal_static(__FUNCTION__); + // Use the advanced drupal_static() pattern, since this is called very often. + static $drupal_static = array(); + isset($drupal_static[__FUNCTION__]) || ($drupal_static[__FUNCTION__] = &drupal_static(__FUNCTION__)); + $script = &$drupal_static[__FUNCTION__]; if (!isset($script)) { // On some web servers, such as IIS, we can't omit "index.php". So, we @@ -4728,7 +4735,10 @@ function drupal_system_listing($mask, $directory, $key = 'name', $min_depth = 1) * keyed array as described above. */ function drupal_alter($type, &$data, &$context1 = NULL, &$context2 = NULL) { - $functions = &drupal_static(__FUNCTION__, array()); + // Use the advanced drupal_static() pattern, since this is called very often. + static $drupal_static = array(); + isset($drupal_static[__FUNCTION__]) || ($drupal_static[__FUNCTION__] = &drupal_static(__FUNCTION__)); + $functions = &$drupal_static[__FUNCTION__]; // Some alter hooks are invoked many times per page request, so statically // cache the list of functions to call, and on subsequent calls, iterate @@ -6214,8 +6224,10 @@ function drupal_check_incompatibility($v, $current_version) { * to return an array with info about all types. */ function entity_get_info($entity_type = NULL) { - // We statically cache the information returned by hook_entity_info(). - $entity_info = &drupal_static(__FUNCTION__, array()); + // Use the advanced drupal_static() pattern, since this is called very often. + static $drupal_static = array(); + isset($drupal_static[__FUNCTION__]) || ($drupal_static[__FUNCTION__] = &drupal_static(__FUNCTION__)); + $entity_info = &$drupal_static[__FUNCTION__]; if (empty($entity_info)) { if ($cache = cache_get('entity_info')) { diff --git a/includes/module.inc b/includes/module.inc index 8df91dd5ca01..a3d6c8b458d1 100644 --- a/includes/module.inc +++ b/includes/module.inc @@ -438,7 +438,10 @@ function module_hook($module, $hook) { * @see module_implements_write_cache(). */ function module_implements($hook, $sort = FALSE, $reset = FALSE) { - $implementations = &drupal_static(__FUNCTION__, array()); + // Use the advanced drupal_static() pattern, since this is called very often. + static $drupal_static = array(); + isset($drupal_static[__FUNCTION__]) || ($drupal_static[__FUNCTION__] = &drupal_static(__FUNCTION__)); + $implementations = &$drupal_static[__FUNCTION__]; // 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 diff --git a/includes/path.inc b/includes/path.inc index 19d52df38d7b..4a9faa373b7e 100644 --- a/includes/path.inc +++ b/includes/path.inc @@ -45,14 +45,21 @@ function drupal_path_initialize() { */ function drupal_lookup_path($action, $path = '', $path_language = '') { global $language; - $cache = &drupal_static(__FUNCTION__, array( - 'map' => array(), - 'no_source' => array(), - 'whitelist' => NULL, - 'system_paths' => array(), - 'no_aliases' => array(), - 'first_call' => TRUE, - )); + // Use the advanced drupal_static() pattern, since this is called very often. + static $drupal_static = array(); + isset($drupal_static[__FUNCTION__]) || ($drupal_static[__FUNCTION__] = &drupal_static(__FUNCTION__)); + $cache = &$drupal_static[__FUNCTION__]; + + if (!isset($cache)) { + $cache = array( + 'map' => array(), + 'no_source' => array(), + 'whitelist' => NULL, + 'system_paths' => array(), + 'no_aliases' => array(), + 'first_call' => TRUE, + ); + } // Retrieve the path alias whitelist. if (!isset($cache['whitelist'])) { @@ -245,7 +252,14 @@ function drupal_get_normal_path($path, $path_language = '') { * not found. */ function arg($index = NULL, $path = NULL) { - $arguments = &drupal_static(__FUNCTION__); + // Even though $arguments doesn't need to be resettable for any functional + // reasons (the result of explode() does not depend on any run-time + // information), it should be resettable anyway in case a module needs to + // free up the memory used by it. + // Use the advanced drupal_static() pattern, since this is called very often. + static $drupal_static = array(); + isset($drupal_static[__FUNCTION__]) || ($drupal_static[__FUNCTION__] = &drupal_static(__FUNCTION__)); + $arguments = &$drupal_static[__FUNCTION__]; if (!isset($path)) { $path = $_GET['q']; @@ -310,7 +324,10 @@ function drupal_set_title($title = NULL, $output = CHECK_PLAIN) { * Boolean value: TRUE if the current page is the front page; FALSE if otherwise. */ function drupal_is_front_page() { - $is_front_page = &drupal_static(__FUNCTION__); + // Use the advanced drupal_static() pattern, since this is called very often. + static $drupal_static = array(); + isset($drupal_static[__FUNCTION__]) || ($drupal_static[__FUNCTION__] = &drupal_static(__FUNCTION__)); + $is_front_page = &$drupal_static[__FUNCTION__]; if (!isset($is_front_page)) { // As drupal_path_initialize updates $_GET['q'] with the 'site_frontpage' path, diff --git a/modules/user/user.module b/modules/user/user.module index 2772b5ffb836..a244ea88740e 100644 --- a/modules/user/user.module +++ b/modules/user/user.module @@ -679,7 +679,10 @@ function user_role_permissions($roles = array()) { */ function user_access($string, $account = NULL) { global $user; - $perm = &drupal_static(__FUNCTION__, array()); + // Use the advanced drupal_static() pattern, since this is called very often. + static $drupal_static = array(); + isset($drupal_static[__FUNCTION__]) || ($drupal_static[__FUNCTION__] = &drupal_static(__FUNCTION__)); + $perm = &$drupal_static[__FUNCTION__]; if (!isset($account)) { $account = $user; -- GitLab