[wp-hackers] apply_filters, merge_filters and reset()

Ryan Boren ryan at boren.nu
Fri Nov 9 17:10:49 GMT 2007


On 11/9/07, Jacob <wordpress at santosj.name> wrote:
> Well, I think cleaning the code up would cut some of the branching,
> which would speed things up. Some things I've noted.
>
> 1. Is the key sorting of $wp_filter[$tag] really necessary? I don't
> think so. Removing the whole merged_filters variable could speed up a
> portion where apply_filters is first called. I would make a guess and
> say that most filters are only called once. Which would boost the speed.
> You would cut the branch (+1), cut out three function calls (reset,
> uksort, and strnatcasecmp), and a few assignments.

Getting rid of it would be nice.  I'm digging through svn history
trying to figure out what the sort was added for.

> 2. Moving the "Do 'all' actions first into its own function will slow
> things down, since it is an additional user function call, but having
> the code in one place would make the code cleaner and any changes
> easier. The cost would be minimal. You could even keep the branch for
> checking and put all the rest of the code in a function. That way, the
> cost for calling the function is only applied if the 'all' hook exists.

Leaving the check and pushing the rest into a function seems
reasonable.  We don't need to optimize the all case too much since it
will only be used in debugging.

> 3. @$wp_current_filters[] = $tag is really damn slow. Not the
> assignment, the '@' character. I don't know what warning or notice it is
> supposed to suppress, but it would be quicker it is was taken out. If
> you need to, just check that $wp_current_filter is an array and make it
> one if it isn't.

Losing the @ is good.  Actually, now that we pass the action/filter
tag to the 'all' hooks, there is less reason to keep track of the
current filter at all.

> 4. Have $wp_current_filter[] = $tag; after the check to see if the tag
> exists. As well as calling the 'all' hook. There is no point of
> executing everything if it doesn't exist. Although, calling the all hook
> might be to say that it could test to make sure that the tag has any
> values, which could be used for debugging, but whether the hook exists
> or not is not directly passed to the all hook, so the hook would have to
> test for it on their its own. (using has_filter).

Okay

> 5. When is $args ever going to be empty? I know, when the all tag
> doesn't exist. However, both need it, so it can be placed outside the
> all hook branch, so that there wouldn't need to be a check for it, which
> would be executed for about 99.99% of users. Moving it from the branch,
> would save some time spent executing 'empty' function. (for
> apply_filters()).

The idea was to call func_get_args only once and make sure we never
call it if neither 'all' nor the current tag have hooks set.  I'm fine
with calling it once for 'all', if set, and once for the current tag
so that we can skip the empty check.  The extra overhead in the 'all'
case is no big deal since 'all' is a debugging scenario.  We should
optimize for the non-'all' case.

Ryan


More information about the wp-hackers mailing list