[wp-trac] [WordPress Trac] #56541: Performance: Don't execute jumps in arrays

[WordPress Trac]

#56541: Performance: Don't execute jumps in arrays
--------------------------+-----------------------------
 Reporter:  Cybr          |      Owner:  (none)
     Type:  defect (bug)  |     Status:  new
 Priority:  normal        |  Milestone:  Awaiting Review
Component:  General       |    Version:  trunk
 Severity:  normal        |   Keywords:
  Focuses:  performance   |
--------------------------+-----------------------------
 To spare CPU time stalling on branch predictors
 ([https://stackoverflow.com/a/9820411 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
 <?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
 <?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
 <?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
 <?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.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/56541>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform