the current forum_cache.patch caches the entire forums object in forum_get_forums. as the documentation notes, this breaks forum_access.

i propose working around this by caching more, smaller objects.

this patch will cache the $counts array, and will cache each $forum object separately. it will also cache the $topics object from forum_get_topics on a per tid, per role, per page basis.

forum access will be respected at the forum level - if the user has no right to see a forum then it won't appear, even though the forum will exist in $counts.

however, any topics which are hidden to normal users but visible to admins might show up in the $counts or $forum objects. perhaps it would make sense to cache these per role.

since my forums are very active, it doesn't make sense for me to invalidate cached objects on update. rather, i'll set an expiry for a fixed time. it should be easy to use the techniques from the cache_node.patch to write invalidation routines if anyone wants to.

CommentFileSizeAuthor
#6 188005.patch10.84 KBfirebus
#2 cache_forum_0.patch12.85 KBfirebus
#1 cache_forum.patch11 KBfirebus
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

firebus’s picture

firebus CreditAttribution: firebus commented
Assigned: Unassigned » firebus
Status: Active » Needs review
FileSize
cache_forum.patch11 KB

i did end up caching per role-combo (using the advcache_array2int function)

also, since i'm caching with expiry set, i commented out the node and comment related cache_clear_all(cache_forum) calls in advcache.module.

i'll report back if there's any good effect from this.

firebus’s picture

firebus CreditAttribution: firebus commented
FileSize
cache_forum_0.patch12.85 KB

oops. you need to cache the pager at the same time that you build the topic list.

your comments_cache does this, but i didn't understand why: the $pager_total global variable is only available at the time pager_query() is called.

this is a little more difficult than comment because the pager is built in a theme function. i decided to expire the cached pager whenever the topics object is cached to make sure they don't get out of sync.

Crimson’s picture

Crimson CreditAttribution: Crimson commented

Subscribing, sounds good.

firebus’s picture

firebus CreditAttribution: firebus commented

the pager caching doesn't respect locale setting. if the topic page is cached by a user in one locale, then the users in another locale will see the wrong language. i'll extend the cache_key to fix this soon.

firebus’s picture

firebus CreditAttribution: firebus commented
Status: Needs review » Needs work
firebus’s picture

firebus CreditAttribution: firebus commented
FileSize
188005.patch10.84 KB

see http://drupal.org/node/183256 for notes on caching the pager. i'm caching the pager in this manner now.

i've also fixed a bug with forum_topic_list_header (it was not getting set on cache_hit)

i don't like the fact that we only need to save the pager once for a given topic, but the current logic sets the pager on each cache miss (once per page of topics). this probably isn't a terrible drain on resources, but there ought to be a better way (there is if you invalidate your tppic cache on each post to a forum, but not one that's immediately obvious to me if you're using timed expiry)

bonus new issue: i'm not respecting the table sort at all so it's entirely broken.