Skip to content

Commit

Permalink
clone query for chunking
Browse files Browse the repository at this point in the history
  • Loading branch information
taylorotwell committed Dec 7, 2016
1 parent 23f5a4c commit 53f97a0
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/Illuminate/Database/Query/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -1847,7 +1847,9 @@ public function chunkById($count, callable $callback, $column = 'id', $alias = n
$lastId = 0;

do {
$results = $this->forPageAfterId($count, $lastId, $column)->get();
$clone = clone $this;

$results = $clone->forPageAfterId($count, $lastId, $column)->get();

$countResults = $results->count();

Expand Down

5 comments on commit 53f97a0

@soundsgoodsofar
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woof. @taylorotwell nbd if you don't like my fix, but cloning the entire builder object each iteration just to back out a where clause? Yeah it works but that perf cost gives me the vapors.

@taylorotwell
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Post your benchmarks.

@vlakoff
Copy link
Contributor

@vlakoff vlakoff commented on 53f97a0 Dec 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was reluctant at first, but I benchmarked this and the clone appears to be super fast. Like, faster than one function call.

@GrahamCampbell
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fast in terms of CPU instructions, but slow in terms of memory allocation.

@GrahamCampbell
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're hitting this loop a lot, there will be a slowdown.

Please sign in to comment.