Menu
With PHP 7.2, each is deprecated. The documentation says:
Warning This function has been DEPRECATED as of PHP 7.2.0. Relying on this function is highly discouraged.
I am working on adapting an e-commerce application and converting all while-each loops to the (supposedly) equivalent foreach.
As you can see below, I have already replaced all the reset & while loops with an equivalent foreach.
It mostly worked fine. However, we had a customer with a very long list of items in her cart that tried to checkout and complained that she gets error 502 from the server.
I tried to reproduce that and found that only her cart fails, it takes over 2 minutes for the checkout page to load, and then times with a 502.
I then started debugging a lot of files I recently modified, trial and error, until I found out that the issue is with this specific file and specific function.
Whenever I switch the first foreach loop back to the while loop, the customer can load the checkout page in less than a second. Switching back to foreach - and again it take minutes, but php times out before it ends execution.
I did perform tests of course on the output of that foreach vs the while loop (var_dump $products_id and $this->contents for example) and they all seem identical. I already rewrote the code to make it work smoothly and to stay PHP 7.2 compatible, but I still can't figure out why this happens.
This is the full function:
function get_content_type() {
$this->content_type = false;
if ( (DOWNLOAD_ENABLED == 'true') && ($this->count_contents() > 0) ) {
// reset($this->contents);
// while (list($products_id, ) = each($this->contents)) {
foreach(array_keys($this->contents) as $products_id) {
if (isset($this->contents[$products_id]['attributes'])) {
// reset($this->contents[$products_id]['attributes']);
// while (list(, $value) = each($this->contents[$products_id]['attributes'])) {
foreach ($this->contents[$products_id]['attributes'] as $value) {
$virtual_check_query = tep_db_query("select count(*) as total from " . TABLE_PRODUCTS_ATTRIBUTES . " pa, " . TABLE_PRODUCTS_ATTRIBUTES_DOWNLOAD . " pad where pa.products_id = '" . (int)$products_id . "' and pa.options_values_id = '" . (int)$value . "' and pa.products_attributes_id = pad.products_attributes_id");
$virtual_check = tep_db_fetch_array($virtual_check_query);
if ($virtual_check['total'] > 0) {
switch ($this->content_type) {
case 'physical':
$this->content_type = 'mixed';
return $this->content_type;
break;
default:
$this->content_type = 'virtual';
break;
}
} else {
switch ($this->content_type) {
case 'virtual':
$this->content_type = 'mixed';
return $this->content_type;
break;
default:
$this->content_type = 'physical';
break;
}
}
}
} elseif ($this->show_weight() == 0) {
// reset($this->contents);
// while (list($products_id, ) = each($this->contents)) {
foreach (array_keys($this->contents) as $products_id) {
$virtual_check_query = tep_db_query("select products_weight from " . TABLE_PRODUCTS . " where products_id = '" . $products_id . "'");
$virtual_check = tep_db_fetch_array($virtual_check_query);
if ($virtual_check['products_weight'] == 0) {
switch ($this->content_type) {
case 'physical':
$this->content_type = 'mixed';
return $this->content_type;
break;
default:
$this->content_type = 'virtual';
break;
}
} else {
switch ($this->content_type) {
case 'virtual':
$this->content_type = 'mixed';
return $this->content_type;
break;
default:
$this->content_type = 'physical';
break;
}
}
}
} else {
switch ($this->content_type) {
case 'virtual':
$this->content_type = 'mixed';
return $this->content_type;
break;
default:
$this->content_type = 'physical';
break;
}
}
}
} else {
$this->content_type = 'physical';
}
return $this->content_type;
}
Thank you
EDIT: here is the array: https://pastebin.com/VawX3XpW
The issue has been tested and reproduced on all the configurations I have tried:
1) High end windows 10 pc + WAMP (Apache 2.4 + MariaDB 10.2 + PHP 5.6+/7+/7.1+/7.2+)
2) High end CentOS/cPanel server + Litespeed + MariaDB 10.1 + PHP 5.6+
Just to emphasize, I am not looking to rewrite the code or emulate each and then rewrite the code, as we won't learn much from that. I am simply trying to find a logical explanation or a way to solve/debug this mystery. Maybe someone, somewhere, ever bumped into such an issue and can shed some light on this.
UPDATE 01/Aug/2018
I have been trying to debug this for days, eventually noticed something interesting. I added "echo points" and exit on the first foreach loop and while loop like so:
function get_content_type() {
$this->content_type = false;
if ( (DOWNLOAD_ENABLED == 'true') && ($this->count_contents() > 0) ) {
// reset($this->contents);
// while (list($products_id, ) = each($this->contents)) { echo '1 ';
foreach(array_keys($this->contents) as $products_id) { echo '1 ';
if (isset($this->contents[$products_id]['attributes'])) { echo '2 ';
// reset($this->contents[$products_id]['attributes']);
// while (list(, $value) = each($this->contents[$products_id]['attributes'])) {
foreach ($this->contents[$products_id]['attributes'] as $value) { echo '3 ';
$virtual_check_query = tep_db_query("select count(*) as total from " . TABLE_PRODUCTS_ATTRIBUTES . " pa, " . TABLE_PRODUCTS_ATTRIBUTES_DOWNLOAD . " pad where pa.products_id = '" . (int)$products_id . "' and pa.options_values_id = '" . (int)$value . "' and pa.products_attributes_id = pad.products_attributes_id");
$virtual_check = tep_db_fetch_array($virtual_check_query);
if ($virtual_check['total'] > 0) {
switch ($this->content_type) {
case 'physical':
$this->content_type = 'mixed'; echo '4 ';
return $this->content_type;
break;
default:
$this->content_type = 'virtual'; echo '5 ';
break;
}
} else {
switch ($this->content_type) {
case 'virtual':
$this->content_type = 'mixed'; echo '6 ';
return $this->content_type;
break;
default:
$this->content_type = 'physical'; echo '7 ';
break;
}
}
}
} elseif ($this->show_weight() == 0) {
// reset($this->contents);
// while (list($products_id, ) = each($this->contents)) {
foreach (array_keys($this->contents) as $products_id) {
$virtual_check_query = tep_db_query("select products_weight from " . TABLE_PRODUCTS . " where products_id = '" . $products_id . "'");
$virtual_check = tep_db_fetch_array($virtual_check_query);
if ($virtual_check['products_weight'] == 0) {
switch ($this->content_type) {
case 'physical':
$this->content_type = 'mixed'; echo '8 ';
return $this->content_type;
break;
default:
$this->content_type = 'virtual'; echo '9 ';
break;
}
} else {
switch ($this->content_type) {
case 'virtual':
$this->content_type = 'mixed'; echo '10 ';
return $this->content_type;
break;
default:
$this->content_type = 'physical'; echo '11 ';
break;
}
}
}
} else {
switch ($this->content_type) {
case 'virtual':
$this->content_type = 'mixed'; echo '12 ';
return $this->content_type;
break;
default:
$this->content_type = 'physical'; echo '13 ';
break;
}
}
} exit; //Exiting from the loop to check output
} else {
$this->content_type = 'physical';
}
return $this->content_type;
}
When I was running the loop with while, the output I got was just "1 13" once, meaning the loop is running only once and stops.
However, when I changed it to foreach, I got a long list of "1 13 1 13 1 13...", meaning it is looping many times. I have gone to investigate further if there is any difference between breaks of while loops and foreach loops, and I still couldn't find any supporting information. I then rewrote the last break; to be break 2; and tested the foreach again and this time it seems to have been running just once, just like when it was a while loop with a break; (not break 2;)
EDIT: Just to clarify - there is no difference between while breaks and foreach breaks. They work the same way.
UPDATE #2:
I have revised } elseif ($this->show_weight() == 0) { to } elseif (2 == 0) { and the while loop now runs as many times as the foreach loop.
var_dump($this->show_weight()); results float 4466.54.
The issue still doesn't make any sense to me.
Thanks again
610
With a simple array of 1,000,000 objects, I iterate over them with foreach and array_map. And surprisingly, foreach ran in 0.24sec average, while array_map took over 3.30sec. Also array_map ran out of memory, I had to do an ini_set("memory_limit","512M"); to at least get some results.
The 'foreach' is slow in comparison to the 'for' loop. The foreach copies the array over which the iteration needs to be performed. For improved performance, the concept of references needs to be used.
The PHP foreach Loop The foreach loop works only on arrays, and is used to loop through each key/value pair in an array.
As a backup solution, What about a drop in replacement performing the same action?
function eachLegacy( &$array )
{
if( current( $array ) !== false )
{
$return = array( 1 => current( $array ), 'value' => current( $array ), 0 => key( $array ), 'key' => key( $array ) ); // Get the current values
next( $array ); // Advance the cursor
return $return;
}
return false;
}
This is actually a very simple algorithmic problem, and it has to do with the fact that when show_weight() is 0 you are looping the same array (edit: and based on your comments, show_weight() itself is also looping the same array).
TL;DR With while all those loops were sharing the same internal pointer and influencing each others. With foreach each loop is independent and therefore it runs way more iterations, hence the performance issue.
Since an example is worth a thousand words, hopefully the following code will make things clearer:
<?php
$array = ['foo','bar','baz'];
foreach ( array_keys($array) as $key ) {
echo $array[$key],"\n";
foreach ( array_keys($array) as $key ) {
echo "\t",$array[$key],"\n";
}
}
echo "---------------\n";
while ( list($key,) = each($array) ) {
echo $array[$key],"\n";
reset($array);
while ( list($key,) = each($array) ) {
echo "\t",$array[$key],"\n";
}
}
This will output:
foo
foo
bar
baz
bar
foo
bar
baz
baz
foo
bar
baz
---------------
foo
foo
bar
baz
As you can see, for an array of size 3, foreach takes 3² iterations, while while takes only 3. That's your performance problem.
while faster?Because at the end of the second (inner) while, the internal pointer of $array would have been pointing to the end of the array, therefore the first (outer) while would simply stop.
With a foreach, since your are using the result of 2 different calls to array_keys, you are using 2 different arrays that do no share the same internal pointer, therefore there is no reason for the loop to stop. A simple return after the second (inner) foreach should solve the problem.
45
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With
