[buddypress-trac] [BuddyPress Trac] #3659: Recently Active widget doesn't scale

[buddypress-trac]

#3659: Recently Active widget doesn't scale
--------------------------------------+---------------------
 Reporter:  mpvanwinkle77             |       Owner:
     Type:  defect (bug)              |      Status:  new
 Priority:  normal                    |   Milestone:  2.2
Component:  Core                      |     Version:  1.2.10
 Severity:  normal                    |  Resolution:
 Keywords:  needs-patch dev-feedback  |
--------------------------------------+---------------------
Changes (by boonebgorges):

 * milestone:  Future Release => 2.2


Comment:

 r-a-y - This is very very cool. Great work. I'm putting it in the 2.2
 milestone.

 Couple thoughts/questions:

 - It looks like you've decided to store all cached instances of a given
 widget together to make invalidation easier. But I think this is less than
 ideal from a performance perspective. With this setup, persistent caches
 will purge all of a widget's cached instances at once (according to
 whatever cleanup routine the admin has set up) instead of one at a time. I
 wonder if we can instead do something like: store each instance in its own
 transient, and then have a single transient that serves as an index for
 the transient keys. Ie

 {{{
 $key = bp_widget_get_transient_key( $class_name, $instance, $args ); //
 unique for each instance
 set_site_transient( $key, $widget_markup );

 $cache_keys = get_site_transient( 'bp_widget_cache_keys' );
 $cache_keys[ $class_name ][] = $key;
 set_site_transient( 'bp_widget_cache_keys', $cache_keys );
 }}}

 Then reference that list of keys to invalidate. Not hugely different from
 what you've suggested, but I think it's better practice to keep the cached
 objects as small as possible.

 - `// as per WP docs, transient keys can only be 40 characters in length`
 - Your technique here will not guarantee unique keys if widget classes
 share the first 35 characters of their names. (Maybe not super likely, but
 think about a hypothetical
 `DPA_Achievements_Widget_Display_Achievements_Of_User` and `_Group`.)
 Maybe run through md5() or something.

 - When applied to BuddyPress, each component should be responsible for
 registering its own cache classes. (I know that doing it centrally doesn't
 hurt anything because those widgets simply won't exist if the component is
 not activated, but still :) )

 - `bp_widget_cache_add_wp_head_hook()` is pretty hacky. Let's just add a
 do_action() hook in `bp_update_user_last_activity()`.

 - Reaching into the `$wp_registered_widgets` global is truly terrible.
 This is not a criticism of you ;)

 Let's circle back after we've run this through its paces on a real site.
 But preliminarily, it looks like this is going to provide huge benefits
 for lots of BP sites!

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/3659#comment:7>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac