Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.x] Optimize Arr::set() method #32282

Merged
merged 1 commit into from
Apr 8, 2020
Merged

[7.x] Optimize Arr::set() method #32282

merged 1 commit into from
Apr 8, 2020

Conversation

nguyenyou
Copy link
Contributor

Same idea as #32192

Replace while:array_shift with foreach:unset. This operation is 3.5x faster. It is arguably more readable as well.

In my isolation tests with 1000 iterations:

  • While loop 1.969ms
  • Foreach loop 0.527ms

I made a small performance test and the result really surprised me. It is a significant performance difference that made me think there's something wrong with my test code.

<?php

function testData() {
  return array_fill(0, 100000, 'TEST DATA');
}

$array = testData();
$start = microtime(true);
$storage = [];
while(count($array) > 1) {
  $element = array_shift($array);
  $storage[] = $element;
}
$whileLoopTime = round(microtime(true) - $start, 4);
$elements = count($storage);
echo "whileloop: {$whileLoopTime} | storage stores {$elements} elements\n";

$array = testData();
$start = microtime(true);
$storage = [];
foreach($array as $i => $element) {
  if(count($array) === 1) {
    break;
  }
  unset($array[$i]);
  $storage[] = $element;
}
$foreachTime = round(microtime(true) - $start, 4);
$elements = count($storage);
echo "foreach: {$foreachTime} | storage stores {$elements} elements\n";

?>

  • 1st run

whileloop: 8.0251 | storage stores 99999 elements
foreach: 0.0145 | storage stores 99999 elements

  • 2nd run

whileloop: 7.9512 | storage stores 99999 elements
foreach: 0.0154 | storage stores 99999 elements

  • 3rd run

whileloop: 7.9988 | storage stores 99999 elements
foreach: 0.0132 | storage stores 99999 elements

@taylorotwell taylorotwell merged commit c5cb62c into laravel:7.x Apr 8, 2020
@nguyenyou nguyenyou deleted the optimize-arr-set branch April 8, 2020 13:46
@browner12
Copy link
Contributor

I'm glad we're seeing these performance improvements, but it got me curious as to why there is such a significant difference.

Turns out array_shift is a time consuming process when you have very large arrays, because every time it is run, it needs to completely re-index the array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants