Posted by jason.fisher on January 10, 2013 at 11:53pm
9 followers
| Project: | Views |
| Version: | 7.x-3.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
I have been investigating 'site lockups' for 30-60 seconds after a view is saved, and I am wondering about this code:
Am I mistaken in thinking that this could cause menu rebuild or cache flushing to run twice in a row? It seems that a menu rebuild that hits _menu is calling views_invalidate_cache, which is then setting menu_rebuild_needed again? So we are potentially invalidating cache at the tail end of a view save and then again when it loads the form action destination URL?
/**
* Implement hook_menu().
*/
function views_menu() {
// Any event which causes a menu_rebuild could potentially mean that the
// Views data is updated -- module changes, profile changes, etc.
views_invalidate_cache();
.. etc ..
}
/**
* Invalidate the views cache, forcing a rebuild on the next grab of table data.
*/
function views_invalidate_cache() {
// Clear the views cache.
cache_clear_all('*', 'cache_views', TRUE);
// Clear the page and block cache.
cache_clear_all();
// Set the menu as needed to be rebuilt.
variable_set('menu_rebuild_needed', TRUE);
// Allow modules to respond to the Views cache being cleared.
module_invoke_all('views_invalidate_cache');
}
Comments
#1
Interesting! We could theoretically just put a debug() into the hook_menu to see what is happening.
#941970: Views rebuilds the menu more than it needs to is also a maybe interesting approach to that problem.
#2
Posting a patch, but we certainly need some manual testing.
#3
I think that makes sense. Enforcing a menu rebuild during a menu rebuild is strange and it's not views responsibility to make sure that caches have been cleared, that's the job of drupal_flush_all_caches() and similar functions. If you just want a menu rebuild then that happens with whatever caches you currently have.
Have not yet tested this.
#4
Tested this and I'm seeing is that the number of views cache rebuilds goes down from 6 to 5 on my site. Which means ~1800 instead of ~2200 views_cache_set() calls.
Which, of course, is still way too much but that's not something to be solved here.
I'm not yet 100% sure why there are so many rebuilds.
#5
Ah, I found the reason for my problem. Missing tables.
Commerce has a default view that includes the address field. We have replaced that with separate fields for our use case. However, that default view is still there and results in multiple attempts to access the views data for it.
Every time this happens, we run into the following else block:
<?phpif (!empty($data->data)) {
$cache[$table] = $data->data;
}
else {
// No cache entry, rebuild.
$cache = _views_fetch_data_build();
$fully_loaded = TRUE;
}
?>
I guess making that elseif (!$fully_loaded) would at least get it down to a single rebuild.
#6
I am seeing another issue related to the block of code you pasted ..
<?php
/**
* Build and set the views data cache if empty.
*/
function _views_fetch_data_build() {
views_include_handlers();
$cache = module_invoke_all('views_data');
foreach (module_implements('views_data_alter') as $module) {
$function = $module . '_views_data_alter';
$function($cache);
}
_views_data_process_entity_types($cache);
// Keep a record with all data.
views_cache_set('views_data', $cache, TRUE);
// Save data in seperate cache entries.
foreach ($cache as $key => $data) {
$cid = 'views_data:' . $key;
views_cache_set($cid, $data, TRUE);
}
return $cache;
}
?>
I have a very field-heavy system (~2000 fields) and this is giving me a views_data cache blob that is 13MB in size. The per-key loop above was added to 'split' the large views_data cache into multiple caches, creating a set of smaller caches that total 13MB in size. Storing the large blob means we now have extraneous traffic on rebuilds. My SELECT SUM(LENGTH(data)) FROM cache_views is ~26MB.
Instead of storing the data twice, why not leave the large views_data:en blob out completely and if needed, rebuild it from: SELECT * FROM cache_views WHERE cid LIKE 'views_data:%:en'?
#7
Testing a solution that just stores keys in views_data cid and then retrieves them from the individual records. A DB SELECT couldn't work because of the multiple cache engine need.
Two functions that change in views/includes/cache.inc:
<?php
/**
* Build and set the views data cache if empty.
*/
function _views_fetch_data_build() {
views_include_handlers();
$cache = module_invoke_all('views_data');
foreach (module_implements('views_data_alter') as $module) {
$function = $module . '_views_data_alter';
$function($cache);
}
_views_data_process_entity_types($cache);
// Keep a record with all views_data cache keys
views_cache_set('views_data', array_keys($cache), TRUE);
// Save data in seperate cache entries.
foreach ($cache as $key => $data) {
$cid = 'views_data:' . $key;
views_cache_set($cid, $data, TRUE);
}
return $cache;
}
function views_cache_get($cid, $use_language = FALSE) {
global $language;
if (variable_get('views_skip_cache', FALSE)) {
return FALSE;
}
if ($use_language) {
$cid .= ':' . $language->language;
}
if ($cid == 'views_data:' . $language->language) {
// views_data cache is now just a reference to views_data cache entries, so let's combine them all into one giant array for legacy cache access
$cache = cache_get($cid, 'cache_views');
$views_cache_keys = $cache->data;
$cache->data = array();
foreach($views_cache_keys as $views_cache_key) {
$single_cache = cache_get('views_data:' . $views_cache_key . ($use_language ? (':' . $language->language) : ''), 'cache_views');
if (isset($single_cache->data)) $cache->data[$views_cache_key] = $single_cache->data;
unset($single_cache);
}
unset($views_cache_key);
return $cache;
} else {
return cache_get($cid, 'cache_views');
}
}
?>
#8
Wasn't worthwhile. Maybe if we had a cache_get_multiple function .. I am going to look into stripping the help text from fields instead?
#9
First things first .. I had a view using a non-existent field that was causing a cache_views views_data rebuild on that page load.
I was able to debug that with a drupal_set_message here:
<?phpfunction _views_fetch_data($table = NULL, $move = TRUE, $reset = FALSE) {
$cache = &drupal_static(__FUNCTION__ . '_cache');
$recursion_protection = &drupal_static(__FUNCTION__ . '_recursion_protected');
$fully_loaded = &drupal_static(__FUNCTION__ . '_fully_loaded');
if ($reset) {
$cache = NULL;
$fully_loaded = FALSE;
}
if ($table) {
if (!isset($cache[$table])) {
$cid = 'views_data:' . $table;
$data = views_cache_get($cid, TRUE);
if (!empty($data->data)) {
$cache[$table] = $data->data;
}
else {
// No cache entry, rebuild.
drupal_set_message('No entry for ' . $cid);
$cache = _views_fetch_data_build();
$fully_loaded = TRUE;
}
}
?>
#10
There is http://api.drupal.org/api/drupal/includes%21cache.inc/function/cache_get... ?
One thing that I did to get the number of tables down in a big project is throw out the field revision tables, assuming you don't need them. That's quite easy in a hook_views_data_alter() implementation and cuts down the size of the whole thing by almost 50%.
I've discussed with @dawehner that would should look into doing something similar as CacheArray implementations in HEAD, that is, only write the whole data initially as a single cache entry and then maintain a frequently used cache entry on which we add all that are explicitly loaded so that we can load them with a single cache get. I will look into implementing that but in 8.x first.
#11
Great tip. A reduction of 1700 entries, with peak memory usage from 170MB to 132MB and page load from 1.7s to 990ms.
If views_ui is disabled, we could cross-reference against admin/reports/fields/views-fields to remove all unused fields?
#12
We probably need two separate cache pools for front-end vs back-end? Front-end with the bare minimum to render views? Back-end with everything as cache_views_ui?
#13
Tested briefly with cache_get_multiple. It seems to be ~0.3s for cache_get_multiple vs 0.18s for the single blob in my case with ~2200 cache_views/views_data:% records, which is probably worthwhile if it avoids max_packet_size or memcache insert size issues.
For reference, the updated functions that I am testing with .. needs some tweaking (will warn on flush/initial rebuild), but was enough to benchmark.
<?php
function _views_fetch_data_build() {
views_include_handlers();
$cache = module_invoke_all('views_data');
foreach (module_implements('views_data_alter') as $module) {
$function = $module . '_views_data_alter';
$function($cache);
}
_views_data_process_entity_types($cache);
// Keep a record with all data keys
views_cache_set('views_data', array_keys($cache), TRUE);
// views_cache_set('views_data', $cache, TRUE);
// Save data in seperate cache entries.
foreach ($cache as $key => $data) {
$cid = 'views_data:' . $key;
views_cache_set($cid, $data, TRUE);
}
return $cache;
}
function views_cache_get($cid, $use_language = FALSE) {
global $language;
if (variable_get('views_skip_cache', FALSE)) {
return FALSE;
}
if ($use_language) {
$cid .= ':' . $language->language;
}
if (1 && $cid == 'views_data:' . $language->language) {
// views_data cache is now just a reference to views_data cache entries, so let's combine them all into one giant array for legacy cache access
$cache = cache_get($cid, 'cache_views');
$time_start = microtime(true);
$views_data_keys = array();
foreach($cache->data as $views_cache_key) {
$views_data_keys[] = 'views_data:' . $views_cache_key . ($use_language ? (':' . $language->language) : '');
}
$full_cache = cache_get_multiple($views_data_keys, 'cache_views');
foreach($full_cache as $key => $value) {
$cache->data[$key] = $value->data;
}
$time_end = microtime(true);
drupal_set_message(sizeof($cache->data) . ' entries retrieved in ' . ($time_end - $time_start) . ' seconds');
return $cache;
} else {
return cache_get($cid, 'cache_views');
}
}
?>