Updated: Comment #10
Problem/Motivation
Too many caches are generated in table cache_views_data in the following case:
- a Views view with more then one display,
- and caching enabled,
- and each display/several displays having the same result, but a different output (e.g. chart and table)
For a complicated view, the results cache and the output cache save notable time.
If a user switches between displays, there is no need to fetch the database data again, if it is already stored in the results cache.
Proposed resolution
Below is a patch that eliminates querying the database a second time, when switching between displays of the same query.
#1055616: Query arguments should be replaced before generating cache ID is a prerequisite, since that issue makes sure the cache key is built correctly.
The display name is discarded from the output key name. This might be a problem in some cases, but IMO only serves documentation purposes.
Comment | File | Size | Author |
---|---|---|---|
#19 | views-result_cache-1606292-19.patch | 538 bytes | Andrew Answer |
|
Comments
Comment #1
johnvPlease find a patch attached.
I am not sure what is better: explicitly add elements from $build_info, (like above),
or explicitly unset($key_data['buildinfo']['title']) and unset($key_data['buildinfo']['breadcrumb'])
Comment #3
johnvIt seems better to unset() data...
Comment #5
johnvI am not sure why the test fails.
The new code from May-18 from #1530740-4: views cache adds all css/js on hits unnecessarily is the cause.
Attached patch is almost identical to #1055616-30: Query arguments should be replaced before generating cache ID, which only considers the hash-data.
[EDIT: disregard this patch. It is in error.]
Only the last line is extra, which removes the $display->id from the results_key.
I am not sure what depends on that piece of key, I suppose it is only to have some human_readable info, and to invalidate cache of a specific display.
Comment #6
johnvresetting status for testbot.
Comment #7
tim.plunkettThings are changing in #1055616: Query arguments should be replaced before generating cache ID enough that this can just be a followup.
Comment #8
johnvAgreed,
then this will be the only change for this request:
ITMT, perhaps we can check if the presence of $this->display->id is obligatory.
Comment #9
tim.plunkettYeah this will need tests.
Comment #10
johnvI found one occasion where this change is not compatible:
Views Data Export generates #1822916: Error 'Base table or view not found:' when VDE-display is batched and cached.
[EDIT] But Views Data Export is not compatible with any cache, and ATM all caching form that module is now programmatically disabled.
Comment #11
johnvComment #12
johnvThis is patch #8 as a file. Unfortunately in svn format.
Comment #13
johnvThis issue was postponed, waiting for #1055616-155: Query arguments should be replaced before generating cache ID to happen. Since that is now committed to 7.x-3.x, this issue can be back on track.
Comment #15
johnvHere is a proper patch.
Comment #16
johnvComment #17
Chris Matthews CreditAttribution: Chris Matthews commentedThe 5 year old patch in #15 to views_plugin_cache.inc does not apply to the latest views 7.x-3.x-dev.
Comment #18
Andrew Answer CreditAttribution: Andrew Answer as a volunteer commentedPatch rerolled.
Comment #19
Andrew Answer CreditAttribution: Andrew Answer as a volunteer commentedPatch rerolled/fixed after last commits.