array_shift in taxonomy.php very slow on large array

[VolodymyrC]

If wordpress blog have a large number posts (100000 or more), then this error has effect:

PHP Fatal error:  Maximum execution time of 30 seconds exceeded in /wpdir/wp-includes/taxonomy.php on line 1961

This is a code from line 1961 in taxonomy.php:

if ( 'id=>parent' == $_fields ) {
	while ( $term = array_shift( $terms ) ) {
		$_terms[$term->term_id] = $term->parent;
	}
} elseif ( 'ids' == $_fields ) {
	while ( $term = array_shift( $terms ) ) {
		$_terms[] = $term->term_id;
	}
} elseif ( 'names' == $_fields ) {
	while ( $term = array_shift( $terms ) ) {
		$_terms[] = $term->name;
	}
} elseif ( 'id=>name' == $_fields ) {
	while ( $term = array_shift( $terms ) ) {
		$_terms[$term->term_id] = $term->name;
	}
} elseif ( 'id=>slug' == $_fields ) {
	while ( $term = array_shift( $terms ) ) {
		$_terms[$term->term_id] = $term->slug;
	}
}

But array_shift function is very slow on large arrays.
​http://kb.ucla.edu/articles/performance-of-array_shift-and-array_pop-in-php

Result:

array_pop takes 0.00089 seconds
array_shift takes 15.15544 seconds
array_reverse + array_pop takes 0.03934 seconds

My suggestion is make a change from array_shift to array_pop.

[SergeyBiryukov]

Originally introduced in [9652], changed in [12729] and [25161].

[jdgrimes]

Just to confirm that array_pop() is faster:

$ php test.php
array_pop takes 0.00221 seconds
array_shift takes 5.69614 seconds
array_reverse + array_pop takes 0.03113 seconds
$ php -v
PHP 5.5.10 (cli) (built: Mar 18 2014 18:10:23) 
Copyright (c) 1997-2014 The PHP Group
Zend Engine v2.5.0, Copyright (c) 1998-2014 Zend Technologies
    with Xdebug v2.2.3, Copyright (c) 2002-2013, by Derick Rethans

I've run into issues with this as well with large taxonomies.

There are quite a few other places in core using array_shift(). Not all of these are subject to huge arrays, but I think they should probably be reviewed.

[boonebgorges]

array_shift() recalculates array indexes, which is why it's so slow on large arrays. See the first couple of comments here: ​http://php.net/manual/en/function.array-shift.php#86783 array_pop() would be fine. We could also use a foreach loop or even wp_list_pluck(), which became available only in 3.1.

[toscho]

I don’t see why we should change the original array at all. We just have to move the pointer, right?

do {
    $term = current ( $terms );
    // add term to new array depending on condition
} while ( next( $terms );

[boonebgorges]

Replying to toscho:

I don’t see why we should change the original array at all. We just have to move the pointer, right?

do {
    $term = current ( $terms );
    // add term to new array depending on condition
} while ( next( $terms );

Right, or foreach. array_pop() seems unnecessary.

[bswatson]

Diff attached that changes

while ( $term = array_shift( $terms ) ) {

to

foreach ( $terms as $term ) {

Looping through via a do-while just seemed more complex and didn't add any benefit since the array is overwritten a few lines later. I also removed the reset just before the block as it's no longer needed if we aren't depending on the array pointer.

[SergeyBiryukov]

Replying to jdgrimes:

There are quite a few other places in core using array_shift(). Not all of these are subject to huge arrays, but I think they should probably be reviewed.

Created #31259 and #31260 for other instances in core and bundled themes.

[SergeyBiryukov]

In 31365:

Remove unnecessary array_shift() usage in get_terms() for better performance.

props bswatson, VolodymyrC.
fixes #31182.