when using memcache, advcache should not serialize anything before calling cache_set, nor does it need to unserialize on cache_get.

CommentFileSizeAuthor
#3 advcache.module.patch760 bytestomchuk
#2 197893.advcache.patch805 bytesfirebus
#2 197893.core_.patch7.85 KBfirebus
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

firebus’s picture

firebus CreditAttribution: firebus commented

i'm using lines like the following:

$map = module_exists('memcache') ? $cache->data : unserialize($cache_data);

however, i think it would be nicer if memcache set a variable on install like 'serialized_cache' that we could query, in case there are other caches that also handle serialization themselves. maybe that's a little too precious, since D6 is about to be released with better handling for various cache backend serialization requirements...

firebus’s picture

firebus CreditAttribution: firebus commented
Status: Active » Needs review
FileSize
197893.core_.patch7.85 KB
197893.advcache.patch805 bytes

i decided to create new, memcache-aware advcache_serialize and advcache_unserialize functions in advcache.module

and replaced all advcache-related calls to serialize and unserialize in the patched modules (currently only block, comment, node, taxonomy since that's all we've currently got working)

i have tested this slightly.

this is pretty minor and straightforward, and won't do anything at all if you're not running memcache.

let me know what you think.

tomchuk’s picture

tomchuk CreditAttribution: tomchuk commented
FileSize
advcache.module.patch760 bytes

Correct me if I'm wrong, but enabling the memcache module (module_exists('memcache')) does not necessarily mean that cache data will be unserialized (especially since memcache.module is blank). I think it would be safer to check the cache_inc variable as in the attached patch.

firebus’s picture

firebus CreditAttribution: firebus commented

iirc, memcache patches the default includes/cache.inc

this is so that, having patched the other core modules that memcache removes serialize() calls from, you can still disable the memcache functionality if you want to without unpatching.

so i still think it makes more sense to check module_exists('memcache'), than to check the settings variables...

neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul commented

firebus, no it requires you use a different cache include setup in your settings.php,

Check the INSTALLATION.txt or scroll halfway down the project page to the colored block to see how this happens.

So actually checking the variable is probably the right solution.

firebus’s picture

firebus CreditAttribution: firebus commented

sigh.

take a look at the patch file.

http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/memcache/pa...

you'll see, towards the top, that cache.inc is modified.

cache.inc is modified in such a way that, having patched it, you no longer want to make calls to serialize and unserialize outside of cache.inc.

yes, having installed memcache, you need to use one of the replacements for cache.inc to get the memcache functionality.

but, whether you're using one of the memcache.inc or the patched cache.inc, you still want to avoid calling serialize and unserialize.

checking module_exists('memcache') is not perfect, but it is better than checking the value of the cache settings variable.