Performance Tweaks for WordPress Trunk Plugin API

[darkdragon]

Clean up Plugin API functions and some optimization tweaks based on WP-Hackers discussion.

• Use Foreach instead of while loops
• Move 'all' hook call to its own function
• Set current filter tag variable after the check for the hook's existence.
• Remove merged_filters variable

[darkdragon]

Based off of r6328 for the Plugin API

[Quandary]

The foreach setup breaks in this implementation, because the priority array is not pre-filled; you'll get a bug if, e.g., a pri-1 callback adds a pri-2 callback on the currently-executing hook, and there are no pri-2 callbacks yet. Please see the second patch attached to bug #5335.

I'm also pretty sure that filters (as implemented for apply_filters) are applicable to strings only. do_action is more generic, but it has a completely separate implementation for that.

[darkdragon]

I can't and will not advocate something so hackish. What about plugins that hook into at priority of '11' or '12'? Plugins that want to execute after everything else would want to hook into priority of 11, as I have done before with 'init'.

If that is the case, then everything else is pointless and should be reverted back, since it has been known to work.

[Quandary]

... because the documentation specifically states that priority levels are 1-10. Anyone doing otherwise is broken.

[darkdragon]

Yeah, then it should lock those numbers in, which it doesn't. It allows for any number, therefore unofficially, any priority number must be supported. The documentation states that to keep things standard, but without checks in the code, then it really is just an suggestion on what the plugin authors should do and not what they must do. If it was something they must do, then an error would have been thrown.

The point is, if I hook into 11, then I mostly know that it will be processed last and if I hook into '0' I can guess that it will be run before anything else.

There is no point arguing this, either we add in the checks or we don't, but unless we do, then filling the priority list manually is not an option.

[Quandary]

You also need to pre-define it if you take out the array sort (I don't see any sorting being done on $wp_filters anymore). Otherwise, the priorities will get all screwed up. Pri-10 will likely wind up running first, because it'll usually be the first level created.

<?php

$example = array();
$example[10] = "This is supposed to be last";
$example[1] = "This is supposed to be first";

foreach($example as $text)
    echo "$text\n";

?>

Output

This is supposed to be last
This is supposed to be first

[darkdragon]

Look at the taxonomy. Apply filters are used on objects. Therefore the documentation is correct.

Replying to Quandary:

You also need to pre-define it if you take out the array sort (I don't see any sorting being done on $wp_filters anymore). Otherwise, the priorities will get all screwed up. Pri-10 will likely wind up running first, because it'll usually be the first level created.

<?php

$example = array();
$example[10] = "This is supposed to be last";
$example[1] = "This is supposed to be first";

foreach($example as $text)
    echo "$text\n";

?>

Output

This is supposed to be last
This is supposed to be first

This the reason I had keywords 'needs-testing', I had thought this is the case. Without testing, it is unknown whether this would be the case. I will submit a new patch that fixes this.

[filosofo]

Replying to Quandary:

... because the documentation specifically states that priority levels are 1-10. Anyone doing otherwise is broken.

Where do you see that? Even the default filters in wp-includes/default-filters.php have much higher numbers than that.

[Quandary]

​Wiki. Apply_filter does not have a corresponding limit. This should go to a new bug; it's tangential to the patch.

[ryan]

We've documented 1 - 10 in the past, but it was never enforced in code and some places in WP use values like 15, 20, 25, and 50. We could probably go to a 0 - 15 scale and set values > 15 to 15. I think 0-15 would cover most current uses. The priorities used in default-filters can be changed to fall in this range. There's no reason for priorities as high as 50.

[Viper007Bond]

The cap can't be 10 as that'd break tons of plugins. Many, many of my plugins need to run after wpautop() slaughters the post for example.

Personally, I think a cap of 20 would be best. This would put the default of 10 right in the middle and allow for enough post-10 room to run enough filters to do most anything in the right order.

[darkdragon]

I think no cap is best, since this is sounding like a WTF (​Worse Than Failure). The story about the guy who kept trying to check the database connection and try to reconnect, up to 5000 times.

I don't think the wiki should be updated. Adding the checks in might hinder plugin developers and would only slow down the Plugin API. I think Plugin developers should give be given faith in that what they've written is what they meant and not mangle and slap their hand when we feel they've done something wrong.

[Viper007Bond]

Yeah, I too personally prefer the flexibility of no limit, but then again, you don't really need 10000 filters/actions.

I think it's worth imposing a cap only if it gives us a performance increase and if that's the case, see my previous comment about the suggested cap.

[ryan]

We need to profile it to see if its worthwhile. Is losing the need to sort the priorities worth it?

[darkdragon]

Replying to ryan:

We need to profile it to see if its worthwhile. Is losing the need to sort the priorities worth it?

No.

[darkdragon]

Optimizations with PHPDoc updates

[darkdragon]

I don't know if you want the PHPDoc stuff, I'll remove it, but it does improve upon some of the stuff from the other bugs (#5163 and #5225). Unintentionally, since I've used the one for the PHPdoc checkout instead of the other checkout (I've so many now, I can't keep track of which ones are broken and what I've used them for).

Basically, it is important since there is a merged filters bug in the do_actions (the $merged_filters isn't being set to global, therefore the test will always fail and decrease performance. Won't break anything, but could slow things down by sorting the array each time.)

I've removed the '@' from the current filter assignment, which should speed things up, since '@' is slow until latest PHP 5.2 version (5.2.3 or 5.2.4 has the performance improvement).

I moved the all tag call to its own function and updated all functions to call it instead. The function has PHPDoc info. You will notice that the hook is not part of the call, I figure using the current_filter function is better.

• I did not use Foreach instead of the do while.
• I did not remove merged_filters
• I did not move current_filters assignment.

All in all, I've only did two of the five things in the WP-Hackers post. The rest were premature.

[darkdragon]

Oh yeah, I use ksort instead of uksort to speed things up a bit (might be premature). I figure it is an edge case that plugin authors will use strings instead of numbers and anyway, it is unknown anyway based on that.

[ryan]

(In [6361]) Filter and action optimizations and phpdoc updates from darkdragon. fixes #5338

[scribu]

Related: #9968