`bp_sort_by_key()` is slow

[boonebgorges]

bp_sort_by_key() uses create_function() to create a usort() callback. This process is very slow, especially when called hundreds of times on a pageload.

See, eg, when pulling up large numbers of activity items -> bp_activity_generate_action_string() -> bp_activity_get_actions(). In my specific case, 70 activity items are being pulled up, which results in bp_activity_get_actions() being called 70 times. Each time the latter function is called, it causes each component's actions to be sorted (another performance issue - I'll open a separate ticket). With 12 components, this results in bp_sort_by_key() being called 840 times.

In the above situation, with the current create_function() implementation, the total wall time for bp_sort_by_key() is about 1,101,000 microseconds - more than 1 second.

Switching to an anonymous function brings the wall time down to about 27,000 microseconds, an improvement of over 400x. But anonymous functions are only available in PHP 5.3. So I'd like to suggest one of the following:

1. We use anonymous functions when available (eg, if ( class_exists( 'Closure' ) ))
2. We break the callback out into its own named function. But then we'll need to put the $key and $type values into a global variable so they're accessible in that function.
3. We fake PHP 5.3 callbacks using a technique like the last one on this page ​http://codeblow.com/questions/pass-extra-parameters-to-usort-callback/

Item 1 seems the most straightforward, but seeing as we don't do this anywhere else in BP, I want to be sure that no one has a problem with the approach.

[DJPaul]

I just tested your first approach because I didn't think it'd get past the parser, and it doesn't work :) ​https://3v4l.org/qutlJ

[boonebgorges]

Replying to DJPaul:

I just tested your first approach because I didn't think it'd get past the parser, and it doesn't work :) ​https://3v4l.org/qutlJ

Oh, you and your technicalities :-D

Two attachments.

1. 6864-global.diff​ implements my suggestion 2 above.
2. 6864-class.diff​ implements suggestion 3.

Neither is as fast as a closure, but both are much faster than create_function(). In my quick benchmarks, approach 6864-class.diff​ is about 10x faster than what we've currently got, while 6864-global.diff​ is 2-3x faster (the buddypress() access is expensive; it may be faster to have standalone globals, but *shudder*)

I'm recommending we go with 6864-class.diff​ unless someone has strong feelings against it.

[DJPaul]

Looks crazy but go for it

[boonebgorges]

In 10514:

Improve performance of bp_sort_by_key().

bp_sort_by_key() originally used create_function() to allow sorting by
dynamic $type and $key. However, create_function() has a tendency to
perform poorly. PHP 5.3 anonymous closures are much faster, but are unavailable
as long as we support PHP 5.2. So we introduce a new class that plays a role
similar to an anonymous closure, with a similar speed benefit.

Fixes #6864.

[boonebgorges]

In 11362:

Use a closure for bp_sort_by_key() callback.

Previously, bp_sort_by_key() was refactored to avoid the use
of create_function(). See #6864. This refactoring needed to be
PHP 5.2-compatible, which required a workaround for the
unavailabilty of closures. This workaround can now be removed.

See #7299.