Opened 15 years ago
Closed 14 years ago
#10264 closed enhancement (wontfix)
use instanceof instead of is_a()
Reported by: | Mittineague | Owned by: | |
---|---|---|---|
Milestone: | Priority: | low | |
Severity: | normal | Version: | 2.8 |
Component: | Performance | Keywords: | needs-patch 2nd-opinion |
Focuses: | Cc: |
Description
My Error Reporting plugin broke with 2.8 in part because of an extreme number of errors (mostly simplepie "non-static called staticly" errors).
But there is also a problem with errors in Core files i.e. is_a()
True they are only E_STRICT deprecated errors and is_a() is un-deprecated in PHP 5.3.0 and it may be more trouble than it's worth to handle the differences for PHP < 5.0, PHP 5.0, PHP 5.1+
I have PHP 5.2.6 so I didn't need the "fatal error" work-around, but I made changes similar to
// if ( !is_a($wp_scripts, 'WP_Scripts') ) $wp_scripts_string = 'WP_Scripts'; if ( !($wp_scripts instanceof $wp_scripts_string) )
in the following wp-includes files
wp-includes/class-feed.php [1 fix] wp-includes/class-IXR.php [5 fixes] wp-includes/classes.php [1 fix] wp-includes/functions.wp-scripts.php [6 fixes] wp-includes/functions.wp-styles.php [5 fixes] wp-includes/general-template.php [1 fix] wp-includes/script-loader.php [4 fixes]
Of particular note is the classes.php file which literally throws hundreds of errors per page on my blog.
function is_wp_error($thing) { // if ( is_object($thing) && is_a($thing, 'WP_Error') ) $wp_error_string = 'WP_Error'; if ( is_object($thing) && ($thing instanceof $wp_error_string) )
And I may be off, but the functions.wp-styles.php file has a function that looks suspicious
function wp_style_is( $handle, $list = 'queue' ) { ..... // if ( !is_a($wp_styles, 'WP_Scripts') ) $wp_scripts_string = 'WP_Scripts'; if ( !($wp_styles instanceof $wp_scripts_string) )
i.e. if $wp_styles is not a WP_Scripts it becomes a WP_Styles
Change History (13)
#1
@
15 years ago
- Component changed from Performance to Warnings/Notices
- Keywords reporter-feedback added
- Milestone changed from Unassigned to 2.9
- Owner Denis-de-Bernardy deleted
#2
@
15 years ago
Thanks for the quick response.
Yes I am "messing" with error reporting. In particular, my Error Reporting plugin which allows for highly customizable (by type/folder) treatment of errors (logging and/or email notification).
I understand the uniqueness of is_a() in that it is needed < PHP 5.0, works in subsequent versions (deprecated, but still works), and is back again in 5.3
My main concerns are in particular it's use in the classes.php is_wp_error function, which is called hundreds of times per page load. My testing shows that is_a is 3 times slower than instanceof (amounts to maybe 1/10 sec. per page load) so it's more a performance thing than an error thing, although in can bloat logs if "ignore_repeated" is false.
I am also wondering about the functions.wp-style.php file's wp_style_is function. i.e.
function wp_style_is( $handle, $list = 'queue' ) { global $wp_styles; if ( !is_a($wp_styles, 'WP_Scripts') ) $wp_styles = new WP_Styles(); $query = $wp_styles->query( $handle, $list ); if ( is_object( $query ) ) return true; return $query; }
I may be missing something, but should WP_Scripts be WP_Styles instead?
#3
@
15 years ago
- Component changed from Warnings/Notices to Permalinks
- Keywords needs-patch added; reporter-feedback removed
- Milestone changed from 2.9 to Future Release
- Owner set to ryan
- Summary changed from wp-includes E_STRICT errors to use instanceof instead of is_a()
- Type changed from defect (bug) to enhancement
Opened #10283 for the second check.
For the switch to instanceof, it'll need to wait until php5.
#4
@
15 years ago
- Component changed from Permalinks to Performance
- Owner changed from ryan to Denis-de-Bernardy
#6
@
15 years ago
I think I may have thought of a better solution then testing for support of the instanceof operator that will still be faster than is_a and not throw errors of any type.
eg.
// if ( !is_a($wp_scripts, 'WP_Scripts') ) if ( !( get_class($wp_scripts) == 'WP_Scripts' ) )
get_class is even a little faster than instanceof and was introduced in PHP version 4
#7
@
15 years ago
for that particular one, is_object() might be a good enough shortcut, too.
get_class() won't deal with inheritance. I'm not entirely sure how much we want to deal with that.
#8
@
15 years ago
Inheritance is a good point. To the best of my knowledge, The 7 files
wp-includes/class-feed.php 'SimplePie' wp-includes/class-IXR.php 'IXR_Date' 'IXR_Base64' 'IXR_Error' wp-includes/classes.php 'WP_Error' wp-includes/functions.wp-scripts.php 'WP_Scripts' wp-includes/functions.wp-styles.php 'WP_Styles' wp-includes/general-template.php 'WP_Styles' wp-includes/script-loader.php 'WP_Scripts' 'WP_Styles'
do not use is_a to test for classes that currently have subclasses in the WordPress Core. Of course this does not mean that they never will, nor that some themes and plugins won't break if inheritance is not taken into account.
Using get_class with is_subclass_of replicates the functionality of is_a
if ( !is_a($wp_scripts, 'WP_Scripts') ) { if ( !( (get_class($wp_scripts) == 'WP_Scripts') || (is_subclass_of($wp_scripts, 'WP_Scripts')) ) ) {
When $wp_scripts is the class, it maintains the performance improvement over is_a, but when $wp_scripts is a subclass, is_sublass_of diminishes the improvement (30% and 80% respectively). Therefore, when a subclass, it's slower than instanceof, but still a bit faster than is_a
As the above code in most cases would only be a very slight improvement (one ten-thousandth of a sec.) at the expense of code readability, perhaps the only file where this should be considered as a replacement is the classes.php file where is_a runs many times per page. This would save about a hundreth to one five-hundreth of a second per page depending on whether or not $thing is a WP_Error or a subclass of WP_Error
*Note, in my comment of last week regarding the classes.php file's is_wp_error function which runs hundreds of times per page, I said 1/10'th sec. per page. This was wrong, it should read 1/100'th sec. per page.
#9
@
15 years ago
- Keywords 2nd-opinion added
Do we want to go through this for 1/100th of a second per page... It's probably useful for wp.com.
Let's get a core dev's feedback...
#11
follow-up:
↓ 12
@
15 years ago
I must admit this turns out to be more complex than I first imagined. There is PHP version support. The instanceof operator requires PHP 5+, but using version_compare() slows things down considerably. Both instanceof and is_subclass_of() have the potential for autoload induced fatal errors and neccessitate a work-around (easy enough I guess, but just the same). Using a work-around, get_class() and is_subclass_of() works in PHP 4,5 and is a bit faster than is_a() but makes the conditional less readable.
So it comes down to a question of a slight improvement in performance vs. code readability. And all this for an E_STRICT error that in ways isn't even a "real" error.
The decision is yours of course, but I think if it was up to me I'd probably wait until WordPress no longer supports PHP 4 and then use the instanceof operator with the work-around.
Thanks for your time and indulgence.
#12
in reply to:
↑ 11
@
15 years ago
Replying to Mittineague:
The decision is yours of course, but I think if it was up to me I'd probably wait until WordPress no longer supports PHP 4 and then use the instanceof operator with the work-around.
Seems like the right thing to do, yeah. :D
is_a() has been re-introduced in php. so not fixing that part.
are you messing around with the error reporting? WP should normally silence E_STRICT.