Opened 13 years ago
Closed 7 years ago
#4076 closed enhancement (maybelater)
Use transients for expensive activity queries
Reported by: | boonebgorges | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Activity | Keywords: | dev-feedback, trac-tidy-2018 |
Cc: |
Description
In light of tickets like #4045, I would like to suggest that we implement some smarter caching of popular and complex activity queries. wp_cache is nice, but it will have no effect on someone not using a persistent object cache. Using transients, on the other hand, will have a small cost in terms of database storage, but a potentially huge payoff even for installations on relatively modest setups.
So, here's what I'm thinking. Cache at the level of bp_has_activities(). Pass everything through a wrapper function (like bp_update_activity_transient()), which will allow admins/plugin devs to decide just how many different kinds of activity queries get cached in this way. By default, cache the following:
- The main activity stream firehose, maybe through the first five pages
- The main activity stream filters (New Blog Post, Updates, etc), first couple pages
- Each user's activity stream, first couple pages
- Each group's activity stream, first couple pages
We can build transient keys out of a hash of the relevant bp_has_activities() parameters.
I will build a proof-of-concept, but I wanted to get feedback as well before digging in too deeply. Thanks.
Change History (4)
#2
@
13 years ago
Thanks for the feedback, DJPaul.
Good point about clearing them. In an ideal world, WP would have a straightforward way to get a list of transient keys; we could then loop through them, and, based on prefix, clear them out. Something like:
$prefix = 'bp_activity_' . $item_type . '_' . $item_id; $transient_keys = wp_get_transient_keys(); foreach( $transient_keys as $key ) { if ( false !== strpos( $key, $prefix ) ) { delete_transient( $key ); } }
We could, in theory, roll this ourselves (by looking either in the options table or in wp_cache, depending on configuration), but it seems hackish.
That said, even if we had to do something less elegant and more piecemeal like storing only a small whitelist of query types, I think it's worth considering, for a couple reasons:
- The vanilla first-20 queries for the main stream, individual users, and individual groups probably constitute a huge percentage of the total number of activity queries on any given site, especially since groups and users default to the activity component. So even just doing them could make a huge dent in the performance problems.
- In my experience and experiments (which is admittedly limited), more complex queries (like those that filter on
type
,user_id
, etc) are actually much faster than the simpler queries, because the indexes used are able to filter out a huge percentage of large datasets. When the only WHERE clauses arehide_sitewide = 0 AND is_spam != 1
, the indexes are nearly useless. So this suggests that the more specific kinds of queries aren't as badly in need of caching attention.
You want to have this as a kind of level of template loop caching, rather than at a lower level in bp_activity_get()
Actually, it wouldn't make a difference if it was at bp_activity_get() or bp_has_activities(). My only point is that it should be at the level of the complex queries, rather than (just) the level of individual activity item caching. So you're probably right that it would be fine there. The pressure points here are the paged_activities and total_activities queries, which get very very expensive on large datasets. Good point about people messing with BP_Activity_Template and the $activities_template global - you're probably right that we should be wary there.
What would happen to existing wp_cache in bp_activity_get()?
Glad you brought that up. Actually, my idea is basically to create a much more sophisticated version of that cache. IMO, bp_activity_sitewide_front caches should be switched to transients anyway, again because it will have more immediate benefit for more different kinds of users.
#3
@
7 years ago
- Keywords trac-tidy-2018 added
We're closing this ticket because it has not received any contribution or comments for at least two years. We have decided that it is better to close tickets that are good ideas, which have not gotten (or are unlikely to get) contributions, rather than keep things open indefinitely. This will help us share a more realistic roadmap for BuddyPress with you.
Everyone very much appreciates the time and effort that you spent sharing your idea with us. On behalf of the entire BuddyPress team, thank you.
If you feel strongly that this enhancement should still be added to BuddyPress, and you are able to contribute effort towards it, we encourage you to re-open the ticket, or start a discussion about it in our Slack channel. Please consider that time has proven that good ideas without contributions do not get built.
For more information, see https://bpdevel.wordpress.com/2018/01/21/our-awaiting-contributions-milestone-contains/
or find us on Slack, in the #buddypress channel: https://make.wordpress.org/chat/
As we'd need to be able to clear these transients pretty easily, and if you build the keys from a range of variables, we're probably limited to the sort of caches that you've bulleted pointed. Things like searching + pagination + max variables would be really challenging to clear when one part of that changes. I think it makes sense and we will end up with a model for anyone implementing Activity into their plugin to copy.
You want to have this as a kind of level of template loop caching, rather than at a lower level in bp_activity_get()? If doing that it seems to me that the transient caching would better go in BP_Activity_Template's constructor. It would also capture anyone creating a BP_Activity_Template object directly, or re-implementing bp_has_activities(), for whatever unusual reason.
What would happen to existing wp_cache in bp_activity_get()?