diff --git includes/bootstrap.inc includes/bootstrap.inc index 0739332..823794e 100644 --- includes/bootstrap.inc +++ includes/bootstrap.inc @@ -730,28 +730,14 @@ function drupal_get_filename($type, $name, $filename = NULL) { * file. */ function variable_initialize($conf = array()) { - // NOTE: caching the variables improves performance by 20% when serving - // cached pages. - if ($cached = cache_get('variables', 'cache_bootstrap')) { - $variables = $cached->data; + $cache = &drupal_static('variable', array()); + if ($cached = cache_get('variable_cache', 'cache_bootstrap')) { + $cache = $cached->data; } else { - // Cache miss. Avoid a stampede. - $name = 'variable_init'; - if (!lock_acquire($name, 1)) { - // Another request is building the variable cache. - // Wait, then re-run this function. - lock_wait($name); - return variable_initialize($conf); - } - else { - // Proceed with variable rebuild. - $variables = array_map('unserialize', db_query('SELECT name, value FROM {variable}')->fetchAllKeyed()); - cache_set('variables', $variables, 'cache_bootstrap'); - lock_release($name); - } + $cache = array(); } - + $variables = $cache; foreach ($conf as $name => $value) { $variables[$name] = $value; } @@ -779,11 +765,48 @@ function variable_initialize($conf = array()) { */ function variable_get($name, $default = NULL) { global $conf; + if (isset($conf[$name])) { + return $conf[$name]; + } + // Use the advanced drupal_static() pattern, since this is called very often. + static $drupal_static_fast; + if (!isset($drupal_static_fast)) { + $drupal_static_fast['cache'] = &drupal_static('variable'); + } + $cache = &$drupal_static_fast['cache']; + + // $cache is populated in variable_initialize() for each request. If it's not + // available here we are in a lower bootstrap phase where the database and + // cache system may not be available yet. + if (isset($cache)) { + // If the variable is not already cached, look it up from the database. + if (!isset($cache['write_cache'])) { + $cache['write_cache'] = TRUE; + } + $value = variable_get_from_storage($name); + $value = isset($value) ? $value : $default; + $conf[$name] = $cache[$name] = $value; + } return isset($conf[$name]) ? $conf[$name] : $default; } /** + * Fetch a variable directly from storage, bypassing any values set in $conf. + * @param $name + * The name of the variable. + * + * @return + * The value of the variable, or NULL if it is not stored in the database. + */ +function variable_get_from_storage($name) { + $result = db_query('SELECT value FROM {variable} WHERE name = :name', array(':name' => $name))->fetchField(); + if ($result) { + return unserialize($result); + } +} + +/** * Sets a persistent variable. * * Case-sensitivity of the variable_* functions depends on the database @@ -803,10 +826,20 @@ function variable_set($name, $value) { global $conf; db_merge('variable')->key(array('name' => $name))->fields(array('value' => serialize($value)))->execute(); - - cache_clear_all('variables', 'cache_bootstrap'); + // On high traffic sites there may be high contention with the variables + // bootstrap cache. Try to avoid rebuilding the cache by reading the current + // cache values, replacing this variable in the cache entry, and writing the + // cache entry back. + if ($cached = cache_get('variable_cache', 'cache_bootstrap')) { + $cache = $cached->data; + if (isset($cache[$name])) { + $cache[$name] = $value; + cache_set('cache_variables', $cache, 'cache_bootstrap'); + } + } $conf[$name] = $value; + variable_reset_cache(FALSE); } /** @@ -828,9 +861,36 @@ function variable_del($name) { db_delete('variable') ->condition('name', $name) ->execute(); - cache_clear_all('variables', 'cache_bootstrap'); - + // On high traffic sites there may be high contention with the variables + // bootstrap cache. Try to avoid rebuilding the cache by reading the current + // cache values, removing this variable from the cache entry, and writing the + // cache entry back. + if ($cached = cache_get('variable_cache', 'cache_bootstrap')) { + $cache = $cached->data; + if (isset($cache[$name])) { + unset($cache[$name]); + cache_set('cache_variables', $cache, 'cache_bootstrap'); + } + } unset($conf[$name]); + variable_reset_cache(FALSE); +} + +/** + * Reset the variable cache. + * + * @param $bootstrap + * Optional parameter to reset the variable cache for bootstrap. If a + * variable is not cached for bootstrap, there is no need to clear the cache. + * Defaults to TRUE. + */ +function variable_reset_cache($bootstrap = TRUE) { + if ($bootstrap) { + cache_clear_all('variable_cache', 'cache_bootstrap'); + } + cache_clear_all('variable_cache:', 'cache', TRUE); + $cache = &drupal_static('variable'); + $cache = array(); } /** @@ -2125,10 +2185,6 @@ function _drupal_bootstrap_database() { function _drupal_bootstrap_variables() { global $conf; - // Initialize the lock system. - require_once DRUPAL_ROOT . '/' . variable_get('lock_inc', 'includes/lock.inc'); - lock_initialize(); - // Load variables from the database, but do not overwrite variables set in settings.php. $conf = variable_initialize(isset($conf) ? $conf : array()); // Load bootstrap modules. @@ -2142,6 +2198,10 @@ function _drupal_bootstrap_variables() { function _drupal_bootstrap_page_header() { bootstrap_invoke_all('boot'); + // Prepare for non-cached page workflow. + require_once DRUPAL_ROOT . '/' . variable_get('lock_inc', 'includes/lock.inc'); + lock_initialize(); + if (!drupal_is_cli()) { ob_start(); drupal_page_header(); diff --git includes/common.inc includes/common.inc index 62a5037..46ad539 100644 --- includes/common.inc +++ includes/common.inc @@ -2565,6 +2565,7 @@ function drupal_page_footer() { drupal_cache_system_paths(); module_implements_write_cache(); system_run_automated_cron(); + variable_write_cache('variable_cache:' . arg(0)), 'cache'); } /** @@ -2590,6 +2591,61 @@ function drupal_exit($destination = NULL) { } /** + * Initialize the variables cache after full bootstrap. + */ +function variable_initialize_full() { + // Allow variables fetched up to this point to be cached in the global + // variable_bootstrap cache. + variable_write_cache('variable_cache', 'cache_bootstrap'); + + // After full bootstrap, we segment the variables cache by system path root. + // For example 'node' or 'admin'. + + $cache = &drupal_static('variable', array()); + if ($cached = cache_get('variable_cache:' . arg(0), 'cache')) { + $cache = $cached->data; + } + $variables = $cache; + + // Ensure existing variables in $conf override cached values. + foreach ($GLOBALS['conf'] as $name => $value) { + $variables[$name] = $value; + } + $GLOBALS['conf'] = $variables; +} + +/** + * Update the variable cache after a cache miss. + */ +function variable_write_cache($cid, $bin) { + $cache = &drupal_static('variable'); + if (!empty($cache['write_cache'])) { + // Get a fresh copy of the variable cache from cache_get(), this will + // include any changes made by other requests between variable_initialize() + // and variable_write_cache(). + if ($cached = cache_get($cid, $bin)) { + $current_cache = $cached->data; + $original_size = count($current_cache); + + $cache = array_merge($current_cache, $cache); + $size = count($cache); + // Only write back to the cache if the size is going to be increased. + // If that's not the case, one of two things may have happened: + // - another process has cached the same set of variables as here, + // so setting it again would be redundant and could add to a stampede. + // - another process has cached a different set of variables that happens + // to be the same size. This is less likely, and just means that the + // next request will cache this set of variables instead of this one. + if ($size >= $original_size) { + unset($cache['write_cache']); + cache_set($cid, $cache, $bin); + } + } + } + drupal_static_reset('variable'); +} + +/** * Form an associative array from a linear array. * * This function walks through the provided array and constructs an associative @@ -4928,6 +4984,7 @@ function _drupal_bootstrap_full() { menu_set_custom_theme(); drupal_theme_initialize(); module_invoke_all('init'); + variable_initialize_full(); } } diff --git modules/simpletest/drupal_web_test_case.php modules/simpletest/drupal_web_test_case.php index 17cbb1f..c0b5102 100644 --- modules/simpletest/drupal_web_test_case.php +++ modules/simpletest/drupal_web_test_case.php @@ -1239,16 +1239,6 @@ class DrupalWebTestCase extends DrupalTestCase { ->condition('test_id', $this->testId) ->execute(); - // Clone the current connection and replace the current prefix. - $connection_info = Database::getConnectionInfo('default'); - Database::renameConnection('default', 'simpletest_original_default'); - foreach ($connection_info as $target => $value) { - $connection_info[$target]['prefix'] = array( - 'default' => $value['prefix']['default'] . $this->databasePrefix, - ); - } - Database::addConnectionInfo('default', 'default', $connection_info['default']); - // Store necessary current values before switching to prefixed database. $this->originalLanguage = $language; $this->originalLanguageDefault = variable_get('language_default'); @@ -1277,6 +1267,16 @@ class DrupalWebTestCase extends DrupalTestCase { file_prepare_directory($temp_files_directory, FILE_CREATE_DIRECTORY); $this->generatedTestFiles = FALSE; + // Clone the current connection and replace the current prefix. + $connection_info = Database::getConnectionInfo('default'); + Database::renameConnection('default', 'simpletest_original_default'); + foreach ($connection_info as $target => $value) { + $connection_info[$target]['prefix'] = array( + 'default' => $value['prefix']['default'] . $this->databasePrefix, + ); + } + Database::addConnectionInfo('default', 'default', $connection_info['default']); + // Log fatal errors. ini_set('log_errors', 1); ini_set('error_log', $public_files_directory . '/error.log'); @@ -1430,7 +1430,9 @@ class DrupalWebTestCase extends DrupalTestCase { */ protected function refreshVariables() { global $conf; - cache_clear_all('variables', 'cache_bootstrap'); + cache_clear_all('variable_cache', 'cache_bootstrap'); + cache_clear_all('variable_cache:', 'cache', TRUE); + drupal_static_reset('variable'); $conf = variable_initialize(); } diff --git modules/simpletest/tests/upgrade/upgrade.test modules/simpletest/tests/upgrade/upgrade.test index 263baff..8c9295c 100644 --- modules/simpletest/tests/upgrade/upgrade.test +++ modules/simpletest/tests/upgrade/upgrade.test @@ -34,6 +34,26 @@ abstract class UpgradePathTestCase extends DrupalWebTestCase { protected function setUp() { global $user, $language, $conf; + // Store necessary current values before switching to prefixed database. + $this->originalLanguage = $language; + $this->originalLanguageDefault = variable_get('language_default'); + $this->originalFileDirectory = variable_get('file_public_path', conf_path() . '/files'); + $this->originalProfile = drupal_get_profile(); + $clean_url_original = variable_get('clean_url', 0); + + // Create test directories ahead of installation so fatal errors and debug + // information can be logged during installation process. + // Use mock files directories with the same prefix as the database. + $public_files_directory = $this->originalFileDirectory . '/simpletest/' . substr($this->databasePrefix, 10); + $private_files_directory = $public_files_directory . '/private'; + $temp_files_directory = $private_files_directory . '/temp'; + + // Create the directories. + file_prepare_directory($public_files_directory, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS); + file_prepare_directory($private_files_directory, FILE_CREATE_DIRECTORY); + file_prepare_directory($temp_files_directory, FILE_CREATE_DIRECTORY); + $this->generatedTestFiles = FALSE; + // Load the Update API. require_once DRUPAL_ROOT . '/includes/update.inc'; @@ -60,31 +80,11 @@ abstract class UpgradePathTestCase extends DrupalWebTestCase { } Database::addConnectionInfo('default', 'default', $connection_info['default']); - // Store necessary current values before switching to prefixed database. - $this->originalLanguage = $language; - $this->originalLanguageDefault = variable_get('language_default'); - $this->originalFileDirectory = variable_get('file_public_path', conf_path() . '/files'); - $this->originalProfile = drupal_get_profile(); - $clean_url_original = variable_get('clean_url', 0); - // Unregister the registry. // This is required to make sure that the database layer works properly. spl_autoload_unregister('drupal_autoload_class'); spl_autoload_unregister('drupal_autoload_interface'); - // Create test directories ahead of installation so fatal errors and debug - // information can be logged during installation process. - // Use mock files directories with the same prefix as the database. - $public_files_directory = $this->originalFileDirectory . '/simpletest/' . substr($this->databasePrefix, 10); - $private_files_directory = $public_files_directory . '/private'; - $temp_files_directory = $private_files_directory . '/temp'; - - // Create the directories. - file_prepare_directory($public_files_directory, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS); - file_prepare_directory($private_files_directory, FILE_CREATE_DIRECTORY); - file_prepare_directory($temp_files_directory, FILE_CREATE_DIRECTORY); - $this->generatedTestFiles = FALSE; - // Log fatal errors. ini_set('log_errors', 1); ini_set('error_log', $public_files_directory . '/error.log');