[wp-trac] [WordPress Trac] #17817: do_action/apply_filters/etc. recursion on same filter kills underlying call

[WordPress Trac]

#17817: do_action/apply_filters/etc. recursion on same filter kills underlying call
-------------------------------------------------+-------------------------
 Reporter:  kernfel                              |       Owner:
     Type:  defect (bug)                         |      Status:  reopened
 Priority:  normal                               |   Milestone:  Future
Component:  Plugins                              |  Release
 Severity:  normal                               |     Version:  2.2
 Keywords:  dev-feedback has-unit-tests has-     |  Resolution:
  patch                                          |     Focuses:
-------------------------------------------------+-------------------------
Changes (by jbrinley):

 * keywords:  dev-feedback has-unit-tests needs-patch 2nd-opinion => dev-
     feedback has-unit-tests has-patch


Comment:

 Went through a lot of iterations before arriving at the version just
 attached in 17817.6.patch.

 Let's start with performance: this is as good as or better than what
 currently exists. Better with 0 callbacks or 3+ callbacks, about the same
 at 1-2 callbacks. Results of the test at
 https://gist.github.com/jbrinley/7518483


 {{{
 Running 10000 times with 0 callbacks
 Total Time (Old): 0.042208
 Average Time (Old): 0.000004
 Total Time (New): 0.037750
 Average Time (New): 0.000004
 New runs in 89.44% of the time of old.

 Running 10000 times with 1 callbacks
 Total Time (Old): 0.212133
 Average Time (Old): 0.000021
 Total Time (New): 0.239030
 Average Time (New): 0.000024
 New runs in 112.68% of the time of old.

 Running 10000 times with 2 callbacks
 Total Time (Old): 0.312370
 Average Time (Old): 0.000031
 Total Time (New): 0.314523
 Average Time (New): 0.000031
 New runs in 100.69% of the time of old.

 Running 10000 times with 3 callbacks
 Total Time (Old): 0.400735
 Average Time (Old): 0.000040
 Total Time (New): 0.384914
 Average Time (New): 0.000038
 New runs in 96.05% of the time of old.

 Running 10000 times with 5 callbacks
 Total Time (Old): 0.604012
 Average Time (Old): 0.000060
 Total Time (New): 0.515111
 Average Time (New): 0.000052
 New runs in 85.28% of the time of old.

 Running 10000 times with 10 callbacks
 Total Time (Old): 1.133043
 Average Time (Old): 0.000113
 Total Time (New): 0.888408
 Average Time (New): 0.000089
 New runs in 78.41% of the time of old.

 Running 10000 times with 15 callbacks
 Total Time (Old): 1.535434
 Average Time (Old): 0.000154
 Total Time (New): 1.266079
 Average Time (New): 0.000127
 New runs in 82.46% of the time of old.

 Running 1000 times with 300 callbacks
 Total Time (Old): 2.447496
 Average Time (Old): 0.002447
 Total Time (New): 2.088821
 Average Time (New): 0.002089
 New runs in 85.35% of the time of old.

 Running 100 times with 3000 callbacks
 Total Time (Old): 0.999711
 Average Time (Old): 0.009997
 Total Time (New): 0.810893
 Average Time (New): 0.008109
 New runs in 81.11% of the time of old.

 }}}

 @wonderboymusic was concerned about the extra time spent running unit
 tests. On my system, running tests against current WP trunk takes about
 3:23 (average of 4 runs). Running with this patch: 3:28 (average of 4
 runs).

 I think the performance concerns have been addressed. Many thanks to those
 of you who kept pushing for more; eventually we got it.

 On to functionality: Anonymous callbacks can be removed. Plugins that
 setup callbacks before WordPress initializes (e.g., batcache, the WP unit
 test suite) will continue to work. Hooks can be called recursively with no
 unexpected side effects. Callbacks can be added and removed mid-run with
 no unexpected side effects.

 I removed a unit test added in 4.0 for ticket #29070. It tested that the
 array pointer for the global `$wp_filter` didn't change when calling
 `has_action()`. Since this new implementation no longer depends on the
 global array pointer, that test was rendered superfluous. The array
 pointer does change, but it doesn't matter.

 I abandoned the custom iterator that characterized my previous patches. No
 matter how much I optimized, it just was not performing as well as I would
 have liked. I was able to squeeze a lot more out of it by keeping the
 management of the iteration within the `WP_Hook` class, at the slight cost
 of a cleaner separation of responsibilities.

 Any questions? Any concerns? Anything holding this back?

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