#58220 closed enhancement (fixed)
Replace usage of substr with str_starts_with and str_ends_with
Reported by: | Soean | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | 6.3 | Priority: | normal |
Severity: | normal | Version: | |
Component: | General | Keywords: | has-patch commit add-to-field-guide |
Focuses: | performance | Cc: |
Description
Replace $needle === substr( $string, 0, $length )
with str_starts_with( $haystack, $needle )
. $length
should be the length of $needle
.
Replace $needle === substr($string, $offset)
where $offset
is negative with str_ends_with( $haystack, $needle )
. The absolute value of $offset
should be the length of $needle
.
It makes the code more readable and use a PHP native function ( which may have a performance benefit ).
Change History (28)
This ticket was mentioned in PR #4395 on WordPress/wordpress-develop by @Soean.
19 months ago
#1
#2
follow-up:
↓ 3
@
19 months ago
These functions were introduced with PHP 8, so before this can be properly considered, WordPress core would need a minimum PHP version of 8.0, which I think is a bit premature still, when looking at the overall PHP version stats.
Alternatively, core would need to create a polyfill for the functions, although possible, that feels like the wrong direction to me. Of course, if there's some substantial performance gains for those running on PHP 8 already, then it would be considered, but this would need to be well documented first :)
#6
@
18 months ago
- Keywords 2nd-opinion added
This seems perhaps premature. See https://core.trac.wordpress.org/ticket/58206#comment:12 and https://core.trac.wordpress.org/ticket/58012#comment:24.
Also, would be good to get some performance testing on PHP 7.4.
@spacedmonkey commented on PR #4395:
18 months ago
#8
@Soean There seems to be a unit test failure for php 8.1
#11
@
17 months ago
- Keywords commit added; 2nd-opinion removed
Looks good to me. Marking as ready for commit.
joelataccelm commented on PR #4395:
17 months ago
#12
wait, how are you doing this and also still claiming to support php 7.4? these aren't supported in php 7.4 and will fatal error.
joelataccelm commented on PR #4395:
17 months ago
#13
@spacedmonkey wait, how are you doing this and also still claiming to support php 7.4? these aren't supported in php 7.4 and will fatal error.
@spacedmonkey commented on PR #4395:
17 months ago
#14
@joelataccelm A backward compatibility shim was added https://github.com/WordPress/wordpress-develop/commit/3cc8f1237a737ccf895682029afacafd7305d420 ( in WP 5.9.0 ). These are already in core in many many places. WordPress currently supports down to PHP 5.6.
joelataccelm commented on PR #4395:
17 months ago
#15
@joelataccelm A backward compatibility shim was added 3cc8f12 ( in WP 5.9.0 ). These are already in core in many many places. WordPress currently supports down to PHP 5.6.
got it, sorry. I just spotted that in the latest compat file. the version I noticed this fatal erroring on doesn't have the str_ functions in the compat file, so I just need to upgrade.
@sabernhardt commented on PR #4395:
17 months ago
#16
According to the Hello Dolly plugin page, it should be compatible with WordPress 4.6 or higher. The str_starts_with
function would require either PHP8 or WP5.9.
#17
follow-up:
↓ 27
@
17 months ago
Performance testing:
Run on PHP 7.4 - 2020 theme and theme test data imported, trunk vs PR. Runned 500 times
Trunk | PR | |
Response Time (median) | 85.6 | 84.93 |
wp-before-template (median) | 20.81 | 20.68 |
wp-before-template-db-queries (median} | 2.99 | 2.97 |
wp-template (median) | 55.52 | 55.22 |
wp-total (median) | 76.42 | 75.98 |
wp-template-db-queries (median) | 5.83 | 5.77 |
Blackfire profiles show a similar story. https://blackfire.io/profiles/compare/73d894ed-7057-44aa-840e-eed31d7a5085/graph
This change is a net benefit. @azaozz
@SergeyBiryukov commented on PR #4395:
17 months ago
#19
Thanks for the PR! Merged in r55990.
#27
in reply to:
↑ 17
@
17 months ago
Replying to spacedmonkey:
Performance testing:
Run on PHP 7.4 - 2020 theme and theme test data imported, trunk vs PR. Runned 500 times
...
This change is a net benefit. @azaozz
Mmmmm, these results are really really interesting :)
So running the PHP native strpos()
is slower than calling a (polifill) function with the exact same strpos()
and with a conditional before it? Will keep it in mind but sounds unbelievable! :)
Trac ticket: https://core.trac.wordpress.org/ticket/58220