Proposal for a new plugin architecture

[FraT]

I'm proposing a new structure of $wp_filter array.

This one can prevents the loops to add and to remove filters.

With this changes the follow code is exec with not relevant differences:

for($i = 0, $j = 100000; $i<$j; $i++)
	add_filter("tag$i", "funtion");

But this is faster:

for($i = 0, $j = 100000; $i < $j; $i++)
	for($i2 = 0, $j2 = 3; $i2 < $j2; $i2++)
		add_filter("tag$i", "funtion$i2");

The core of my proposal is to change the add_filter function with this one:

function add_filter($tag, $function_to_add, $priority = 10, $accepted_args = 1) {
	global $wp_filter;

	$wp_filter[$tag][$priority][serialize($function_to_add)] = array('function' => $function_to_add, 'accepted_args' => $accepted_args);
	return true;
}

[FraT]

An example of wp-includes/plugin.php modified to the $wp_filter structure that I'm proposing

[markjaquith]

Previous file as a patch

[markjaquith]

Patch uploaded without endorsement. Haven't had time to grok the changes. But as long as it is 100% backwards compatible and is faster, it should be good for inclusion. It sure is less code! What say you, Ryan?

FraT, do you have any numbers comparing the speed differences?

[ryan]

If it is faster and doesn't break anything, I'm down. Performance numbers would indeed be interesting.

[ryan]

I did some profiling with xdebug and cachegrind. It seems to be faster and reduces the number of function calls made by apply_filters().

+1

[markjaquith]

Excellent. You did the research, so you can have the honors.

I like tickets like this... more please!

[ryan]

(In [4955]) add and apply filter optimizations from FraT. fixes #3875

[weyhan]

I would like to point out that the change actually did break something.

I have post the details at the forum ​http://wordpress.org/support/topic/124805?replies=2

Please have a look and let me know if my accessment is correct.

[daviding]

Does this mean that the code will be reverted to as it was before the code was changed?

It may be more efficient, but I really miss using Post Teaser ... which stopped working at Wordpress 2.2.1.

[santosj]

It is simple to fix.

Just check to see that $function_to_add is an array and if it is, then take the first key ( $function_to_add[0] ) and see if it is an object or a string. If it is an object, then get the name ( get_class($function_to_add[0]) ) and use that for the serialized part.

It may decrease the speed a little bit, but it will fix the previous issues. And actually, for plugins and internal code that uses functions, it won't affect the speed at all (or so in my humble opinion).

function add_filter($tag, $function_to_add, $priority = 10, $accepted_args = 1) {
    global $wp_filter;

    $serialized = &$function_to_add;
    if(is_array($function_to_add))
    {
        if(is_object($function_to_add[0]))
        {
            $serialized = array(get_class($function_to_add[0]), $function_to_add[1]);
        }
    }

    $wp_filter[$tag][$priority][serialize($serialized)] = array('function' => $function_to_add, 'accepted_args' => $accepted_args);
    return true;
}

In theory, this should work.

[santosj]

Well, I suppose the multiple if statements are not needed. If speed is what you are looking for then it should be slightly faster than with both of them. I suppose, since it would fail with the first statement and not evaluate the second once it sees that the two are AND.

Replying to santosj:

function add_filter($tag, $function_to_add, $priority = 10, $accepted_args = 1) {
    global $wp_filter;
 
    $serialized = &$function_to_add;
    if(is_array($function_to_add) && is_object($function_to_add[0]))
    {
        $serialized = array(get_class($function_to_add[0]), $function_to_add[1]);
    }

    $wp_filter[$tag][$priority][serialize($serialized)] = array('function' => $function_to_add, 'accepted_args' => $accepted_args);
    return true;
}

[fedallah]

When I replied to the Trac email on this post earlier in the month, I figured the list would thread my response to Trac, too. Oops. So here's a link to the wp-hackers email about this bug for reference:

​http://comox.textdrive.com/pipermail/wp-hackers/2007-July/013734.html

Anyway, re: santosj, the get_class() approach fails whenever an object is instantiated more than once, and each instance is attempted to be registered with the same method. I wrote about this in my message to the list earlier this month.

Example code:

class Foobar {
	var $name;
	
	function Foobar( $name ){
		$this->name = $name;
	}
	
	function hook( $v ) {
		return $this->name . " Hooked! $v";
	}
}

$foo1 = new Foobar("baz");
$foo2 = new Foobar("quux");

create_filter( 'the_content', array( &$foo1, "hook" ) );
create_filter( 'the_content', array( &$foo2, "hook" ) );

$foo1 and $foo2 will have the same class name, and therefore, will be the same index in the filter array. So sadly, this solution doesn't work. See my post for one which I think has a better shot.

Also, serialize is big and expensive. I think we should avoid calling it.

[santosj]

Thanks, I sent a reply to the mailing list.

[santosj]

Patch for review, hasn't been tested, but might provide fixes.

[santosj]

Fixes issues in previous diff, overwriting.

[santosj]

The new patch is slower than the first patch, but I have been able to confirm that it fixes the issues brought by serialize.

You can improve the speed by moving the code from the _wp_filter_build_unique_id() to the add_filter and remove_filter code. It is only about ~10-13 ms improvement with 2000 add_filter() calls.

[santosj]

Plugin Unit Test

[darkdragon]

Optimized patch to decrease time by one half with twice as much

[darkdragon]

Time Decreased by new patch is as follows:

2000 apply filters with old, wp-includes.plugin.php.2.diff, file took 83 ms total.
4049 apply filters with new, plugin.diff.php, takes ~40 ms total.

serialize method takes less than 20 seconds with 4000 apply filters.

The old method does not work, the new patch does work with classes. Is the small decrease in speed worth not having this patch?

[darkdragon]

Proof of Concept of Bug and Proof that Patch works! If you get the not so friendly message, then the current plugin.php needs to be patched.

[darkdragon]

Added not so friendly plugin which tests gives the proof of concept for bug and patch. I could make it more friendly, but hey my working copy works fine.

Disclaimer: WordPress does not suck and I'm only being sarcastic. Any hurt feelings, shouldn't not be reflected in words or physical contact.

[darkdragon]

Remove from admin panel too and more kinder words.

[ryan]

(In [5936]) Fix how wp_filter array is keyed. Props santosj/darkdragon. fixes #3875 for 2.3

[ryan]

We might be able to get this in 2.2.3. Anyone want to port the patch to 2.2?

[santosj]

Working patch for WordPress 2.2.x branch. Code is similar.

[darkdragon]

Testing the new patch, allows the test plugin to function correctly for WordPress version 2.2.2. Code is slightly different than patch for 2.3.

[foolswisdom]

I was looking at this as a non-severe change to a maintenance release, because the ticket only mentions improved performance. Ryan explained to me that the current 2.2 behavior "breaks quite a few plugins".

[darkdragon]

Well, it only breaks the ones that choose to use objects. There are ways that objects can get around the limitations with the break. I have recommended in the past that a Registry Pattern would solve the problem.

What is fixed in the patch is the expectation that nothing changed in the class. Normally, this would never be the case. Most plugins that I looked at use the Singleton pattern, which suffers the most.

There are several other fixes that can be applied to the singleton pattern that would also fix this issue ( sleep() method ).

[ryan]

(In [6014]) Fix how wp_filter array is keyed. Props santosj/darkdragon. fixes #3875 for 2.2