Skip to:
Content

BuddyPress.org

Opened 9 years ago

Closed 9 years ago

#6866 closed defect (bug) (wontfix)

`bp_activity_get_post_types_tracking_args()` is called too frequently

Reported by: boonebgorges's profile boonebgorges Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Activity Keywords:
Cc:

Description

bp_activity_get_post_types_tracking_args() is called every time bp_activity_get_actions() is called. This can be hundreds of time per page. See #6864, #6865.

My profiling shows that the call to apply_filters() in this function can cause performance problems when called too many times.

Attachments (1)

6866.diff (1.9 KB) - added by boonebgorges 9 years ago.

Download all attachments as: .zip

Change History (5)

@boonebgorges
9 years ago

#1 @boonebgorges
9 years ago

6866.diff is one possible way of streamlining things: only allow post type tracking args to be set up one time, stashing the processed results in a static. In my testing, this dramatically increases performance. However, there are a couple of potential problems:

  1. It's possible that someone may be using a targeted filter to modify 'bp_activity_get_post_types_tracking_args' at different points in the pageload, so that a callback applied late in the pageload would not be appropriate the first time the function is called.
  2. Post types may not register themselves in time for the first call to bp_activity_get_post_types_tracking_args(), and as a result, would not be included in the static data.

@imath Pinging you for any thoughts :-D

#2 @imath
9 years ago

To be honest i'm feeling pretty insecure here. My first thought is "we need to wait till all post type registered"

something like

function bp_inited() {
    do_action( 'bp_inited' );
}
add_action( 'init', 'bp_inited', 10000 );

And in the function something like
if ( ! isset( $args ) || ! did_action( 'bp_inited' ) ) {

#3 @boonebgorges
9 years ago

Depending on a very late hook doesn't seem safe. Conventionally, plugins register post types at 'init', but there's no reason they can't be registered at 'wp' or even later.

It would be possible to run the filter only if we've detected that the dynamically-built $post_types_tracking_arms has changed since the last time the function was run. Since apply_filters() is what's introducing the overhead, this would probably help a lot. However, I don't really like the idea of running a filter intermittently - this kind of pattern is really hard for devs to debug and modify.

Let's wait to see how the performance improvements suggested in #6864 and #6865 turn out. It's likely that they will offset most of what I'm suggesting here, and we can wontfix this ticket.

#4 @boonebgorges
9 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

As I thought, the improvements in #6864 and #6865 are very dramatic, and make me think that we no longer need to pursue this. Thanks for entertaining the thought :)

Note: See TracTickets for help on using tickets.