Opened 11 years ago
Closed 8 years ago
#5407 closed enhancement (fixed)
Improvements to object caching in bp-groups
Reported by: | boonebgorges | Owned by: | |
---|---|---|---|
Milestone: | 2.0 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Groups | Keywords: | |
Cc: |
Description
The groups component uses object caching in some places, but it's incomplete and in some cases appears to be incorrect. This is a general ticket for commits that address the issue for the 2.0 cycle.
Change History (15)
#5
@
11 years ago
I think r8001 is incorrect. We only want to cache complete group objects, not partials. We do cache the group already on line 223 already, after admins and moderators are added.
It's probably worth discussing how we query for and cache supplemental component data, both including and excluding meta.
#6
@
11 years ago
Good call.
I was getting cache misses when grabbing the groups object for secondary avatars in the activity loop.
Will revert and look into it some more.
#7
@
11 years ago
johnjamesjacoby is correct. The caching happens here: https://buddypress.trac.wordpress.org/browser/trunk/bp-groups/bp-groups-classes.php?rev=8001#L223 We could do multiple layers of caching here if we thought it was worth the trouble (one for basic data, and another for populate_extras data). My thought is that it's *not* currently worth the trouble (which is why it currently works the way it does), but I'd be happy to discuss.
It's probably worth discussing how we query for and cache supplemental component data, both including and excluding meta.
Metadata - in the technical sense of "stored in the meta tables" - should never be cached with the object. We have separate object caches for that.
In an ideal world, we would have fine-grained caches for all other supplemental data - stuff like admins_mods_of_group_54. But this can be a lot of work, because it multiplies the amount (and different kinds) of invalidation we have to do. So, where it's easier, like in this case, I think it's fine to cache with the object.
r-a-y - Just saw your point about secondary avatars. That would be the one place where populate_extras=false caching might make sense. If you feel like breaking it up into two layers of cache, please be my guest. (Please write unit tests for it :) )
#8
@
11 years ago
That would be the one place where populate_extras=false caching might make sense. If you feel like breaking it up into two layers of cache, please be my guest.
Yup, that's what I plan on doing. And yes, I'll write some unit tests for this :)
#10
@
11 years ago
- Milestone changed from 2.0 to 2.1
The groups component still needs a fair amount of attention wrt to object caching, so I'm moving this to 2.1 for further work.
#12
@
10 years ago
See r8610. I think we now can move forward with some of the changes we'd initially discussed (removing the direct queries for admin/mod info, and using BP_Group_Member_Query
instead)
#15
@
8 years ago
- Milestone changed from Future Release to 2.0
- Resolution set to fixed
- Status changed from new to closed
Closing this one.
We've done quite a bit of improvements to group caching over the past few releases.
Moving this back to the older 2.0 milestone where the initial improvements were implemented.
In 7956: