[wp-trac] [WordPress Trac] #58291: Speed up WordPress hooks addition/removal by ~20%

[WordPress Trac]

#58291: Speed up WordPress hooks addition/removal by ~20%
--------------------------+------------------------------
 Reporter:  bor0          |       Owner:  (none)
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  Awaiting Review
Component:  Plugins       |     Version:  trunk
 Severity:  normal        |  Resolution:
 Keywords:  has-patch     |     Focuses:  performance
--------------------------+------------------------------

Comment (by SergeyBiryukov):

 Thanks for the patch! I would suggest evaluating each optimization
 separately:

 Replying to [ticket:58291 bor0]:
 > - Calling a static method than a function is faster - check `static-
 faster.php` for proof
 > - Removed unused arguments, passing less data to the stack - speed
 should be obvious here
 > - If the argument is an object, return `spl_object_id` right away rather
 than constructing a pair where the second element is `''` and then
 concatenate that empty string - speed should be obvious here

 This makes sense to me.

 > - [https://github.com/php/php-
 src/blob/7c44baa/ext/spl/php_spl.c#L661-L672 spl_object_id] is faster than
 [https://github.com/php/php-src/blob/7c44baa/ext/spl/php_spl.c#L649-L659
 spl_object_hash], as we are not calling [https://github.com/php/php-
 src/blob/7c44baa/ext/spl/php_spl.c#L674-L677 strpprintf].

 If a polyfill is needed for PHP < 7.2, this might have to wait until the
 minimum required version is bumped in #57345, otherwise it may negatively
 affect performance on those older versions. See comment:12:ticket:58206
 and comment:24:ticket:58012 for similar recent discussions.

 >   - Sometimes `WP_Hook::build_unique_id` will return a string (when
 concatenated), and sometimes an integer, but this should be okay.

 This would need to be explained in the `@return` tag. Would it be
 preferable to always cast the result to a string for consistency?

 > - No need to concatenate `'::'` - speed should be obvious here, fewer
 strings to concatenate

 I think this is still needed for canonical callback representation, see
 [24251] / #23265.

 > - Bail early on `add_filter` if `$idx` is null - speed should be obvious
 here

 I might be missing something, but if `::build_unique_id()` always returns
 a string or an integer, when exactly would it be `null` and how common
 might that be? This adds a return value to `::add_filter()`, which does
 not currently have one, for a case that is not quite clear to me yet.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/58291#comment:2>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform