cPanel Question posted in
The official documentation can be seen here.

While each is deprecated, Foreach replacement takes a lot more time – CPanel

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

Tags:

4

Answers


  1. 0

    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;
}
    Login or Signup to reply.
  2. 0

    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.

    Why was 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.

    Login or Signup to reply.
  3. 0

    I don’t quite understands the main problem here but, you can start by optimizing your foreach loops using key value approach:

    foreach($this->contents as $products_id => $products_value) {
    echo '1 ';
    if (isset($products_value['attributes'])) {
        echo '2 ';
        foreach ($products_value['attributes'] as $value) {
            echo '3 ';
            ...

    using return in switch will break the whole function, exit also.
    if you want to break a loop from a switch statement use:

    break NUMBER_OF_PARENT_STATEMENTS;

    break 2; will break switch and the parent foreach

    break 3; will break switch and the first and the second parent foreach

    Login or Signup to reply.
  4. 0

    In your original code, if the following returns true in the first iteration:

    $this->show_weight() == 0

    The code loops through $this->contents using each(), setting the array pointer on $this->contents to the end. So when we get back around to the first while() statement, it assumes it’s already done, because the next call of each($this->contents) returns false.

    Login or Signup to reply.
Please signup or login to give your own answer.

Back To Top
Search