[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):

 Replying to [comment:4 knutsp]:
 > > 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.
 >
 > That was PHP < 8. For < 7.2 it's only about 10%. Then 90%, increasing
 week by week, may get better performance.

 Yes, you're right, I was having second thoughts about this too :) It
 should be fine to use `spl_object_id()` here, with a polyfill for PHP <
 7.2.

 Replying to [comment:5 bor0]:
 > > > - 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.
 >
 > You are correct. We should just `return` - I followed the same approach
 from `::has_filter`. If we want to avoid bailing out early, I guess we can
 remove it from `::has_filter` as well?

 Thanks for the clarification, tracked this down to the initial
 implementation of `has_filter()` in [6320] / #5231.

 Yeah, it does not seem necessary there either if `::build_unique_id()` can
 never return a falsey value in normal usage.

 > Basically, the function seems to have some holes - could be worth
 figuring out how to refactor it so that it covers these cases as well:
 >
 > {{{
 > wp> _wp_filter_build_unique_id( null, null, null );
 > PHP Warning:  Undefined array key 0 in /usr/local/var/www/wp-
 includes/plugin.php on line 1000
 > Warning: Undefined array key 0 in /usr/local/var/www/wp-
 includes/plugin.php on line 1000
 > PHP Warning:  Undefined array key 0 in /usr/local/var/www/wp-
 includes/plugin.php on line 1003
 > Warning: Undefined array key 0 in /usr/local/var/www/wp-
 includes/plugin.php on line 1003
 > => NULL
 > }}}
 >
 > I understand null is not expected to be used with `add_filter`, but
 bailing out seemed like a reasonable defensive choice.

 I would say the function should only accept parameters of the documented
 type. It appears to be primarily intended for the plugin API where it is
 always called with the correct parameters, and should not be used on its
 own, so I think the warnings for invalid input would be expected here.

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