#56541 closed enhancement (fixed)
Performance: Don't execute jumps in loops
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.3 | Priority: | normal |
Severity: | normal | Version: | |
Component: | General | Keywords: | has-patch commit |
Focuses: | performance | Cc: |
Description
To spare CPU time stalling on branch predictors (briefly explained), it's better to execute as few jumps in the code, especially when we can easily modify them. The simplest form of implementing a jump in PHP is via an if
-statement. Executing those inside loops will increase CPU time quickly.
For example, and probably the most significant, in wpdb::get_results()
, the following code block could be converted to the next code block.
Current:
<?php foreach ( (array) $this->last_result as $row ) { if ( ARRAY_N === $output ) { // ...integer-keyed row arrays. $new_array[] = array_values( get_object_vars( $row ) ); } else { // ...column name-keyed row arrays. $new_array[] = get_object_vars( $row ); } }
My proposal:
<?php if ( ARRAY_N === $output ) { foreach ( (array) $this->last_result as $row ) { // ...integer-keyed row arrays. $new_array[] = array_values( get_object_vars( $row ) ); } } else { foreach ( (array) $this->last_result as $row ) { // ...column name-keyed row arrays. $new_array[] = get_object_vars( $row ); } }
Yes, that is more code outputting the same data, but it executes faster. To get an indication of the time saved when querying 2000 items, see https://3v4l.org/KvpIR#tabs.
Across the board, there's about a 10% performance improvement for the if
branch, and 20% for the else
branch. When we loop over 80,000 results, which I don't think is out of the ordinary, the results become significant: https://3v4l.org/6OOHn#tabs. This is by optimizing just one foreach
loop. Applying these savings everywhere could yield not only measurable but noticeable improvements.
I found another example at WP_Upgrader_Skin::error()
, withal which keeps calling the same WP_Error::get_error_data()
method up to three times per loop:
<?php foreach ( $errors->get_error_messages() as $message ) { if ( $errors->get_error_data() && is_string( $errors->get_error_data() ) ) { $this->feedback( $message . ' ' . esc_html( strip_tags( $errors->get_error_data() ) ) ); } else { $this->feedback( $message ); } }
Therefor my proposal, to which I added a trim()
call to spare us an if-statement all the same:
<?php $error_data = $errors->get_error_data(); $error_data = is_string( $error_data ) ? esc_html( strip_tags( $error_data ) ) : ''; foreach ( $errors->get_error_messages() as $message ) { $this->feedback( trim( "$message $error_data" ) ); }
The principle is this: Do not execute work inside a loop that should be done outside of it.
Change History (11)
#1
@
2 years ago
- Summary changed from Performance: Don't execute jumps in arrays to Performance: Don't execute jumps in loops
#2
@
2 years ago
- Keywords needs-patch added
- Type changed from defect (bug) to enhancement
- Version trunk deleted
#3
@
2 years ago
It caught my eye as I needed to study the code.
I found the second example as I tried to create a crude lookup "tool" for it using regular expressions, but it doesn't work well in JS, plus it yields many false positives in VSCode: https://regex101.com/r/nkZIUZ/2.
#4
@
2 years ago
The results speak for themselves IMO. Nice work @Cybr!
The principle is this: Do not execute work inside a loop that should be done outside of it.
Should this be considered for an entry in the handbook, I would revise this to:
Do not execute work inside a loop that can be done outside of it.
That way, personal opinion regarding "should" doesn't come into it.
#6
@
20 months ago
- Milestone changed from Awaiting Review to 6.4
- Owner set to spacedmonkey
- Status changed from new to reviewing
This ticket was mentioned in PR #4719 on WordPress/wordpress-develop by @spacedmonkey.
20 months ago
#7
- Keywords has-patch added; needs-patch removed
Trac ticket: https://core.trac.wordpress.org/ticket/56541
#10
@
20 months ago
- Keywords commit added
@Cybr I committed the database tweak you noted. If you have any other examples, like the get_error_data
, let's do these as other smaller tickets in WP 6.4.
@spacedmonkey commented on PR #4719:
20 months ago
#11
Committed.
@Cybr 👍 Do you use an automated tool to identify these or is it manual?