Caches are constantly rewritten when empty [#1247412]

When context module is installed but no contexts are defined, it stores empty arrays or even NULL in the cache. On the next request, these caches are incorrectly ignored, re-loaded and re-saved into the cache. This happens on every request.

In case of the conditions_map cache, this is actually repeated ~8 times *per page load*.

Since writing a cache is relatively slow, this seriously impacts performance.

The attached patch assures that:
- an empty array is stored into the cache (instead of NULL)
- changes both context_cache_get() to return empty arrays (by replacing !empty with isset) and changes the calling code to accept empty arrays.

Comment	File	Size	Author
	cache_fixes.patch	1.13 KB	Berdir

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Berdir

German

Switzerland

CreditAttribution: Berdir commented 23 August 2011 at 12:21

Status:

Active

» Needs review

Comment #2

emanaton CreditAttribution: emanaton commented 28 January 2014 at 00:11

Issue summary:	View changes
Status:	Needs review	» Closed (cannot reproduce)

It appears that this bug is no longer effecting the project. I've taken the following steps:

Created fresh install of Drupal 7.26
Installed CTools v. 1.3 and Context v. 3.1
Enabled the CTools and Context modules
Added debug-messages (drupal_set_message) to the context_cache_get() function to alert when cache is fetched or cache is determined to be empty on fetch.
Observed the expected single-fetches of cache before and after contexts are added.

Barring further activity on this thread, I'm closing out this ticket on the assumption that this is an old bug that effected the beta release, but which no longer effects the current release.

Comment #3

rooby CreditAttribution: rooby commented 28 January 2014 at 01:34

Status:

Closed (cannot reproduce)

» Needs review

I have definitely seen this issue on a live site although it was a little while ago.

I would hesitate to close the issue though because if you look at the code in the patch it would seem like a sensible change regardless.

If a maintainer still doesn't want to commit it though I won't argue.

Comment #4

Berdir

German

Switzerland

CreditAttribution: Berdir commented 28 January 2014 at 07:26

Based on the code, I expect this still to be a problem. Try to add a drupal_set_message() in cache_set(), not get(). The static cache will prevent multile cache_get() calls but it should still try to save empty caches back on every request.

Patch also still applies...

Comment #5

emanaton CreditAttribution: emanaton commented 28 January 2014 at 13:52

Assigned:

Unassigned

» emanaton

Comment #6

emanaton CreditAttribution: emanaton commented 28 January 2014 at 15:53

Greetings rooby and Berdir,

You're right!

On a fresh install without modifications (other than adding debugging messages), I'm detecting the following call with each page load:

context_cache_get - NO CACHE - registry
context_cache_get - registry
context_cache_get - condition_map
context_cache_get - context
context_cache_set - context
context_cache_set - condition_map
context_cache_get - block_info

... but after applying the patch:

context_cache_get - NO CACHE - registry
context_cache_get - registry
context_cache_get - condition_map
context_cache_get - block_info

On re-reading the original description, I see that I should have figured out to be testing cache_set as well. I shouldn't have been testing code so late at night, me thinks. =oP

I'm going to do a bit of regression testing on this in the next few days to make sure there's no unforeseen side effects from this; I don't expect any based on the code, but better to be safe. Thank you both for commenting so quickly and getting me back on track!