loop optimization #757
loop optimization #757
Conversation
5e8f95d to
01efeea
Compare
|
Could you profile whether its faster after the change? |
|
| if ($arraysToProcess[$j]->isKeysSupersetOf($arraysToProcess[$i])) { | ||
| $arraysToProcess[$j] = $arraysToProcess[$j]->mergeWith($arraysToProcess[$i]); | ||
| array_splice($arraysToProcess, $i--, 1); | ||
| $arraysToProcessCount--; |
There was a problem hiding this comment.
this does change the loop limit boundary, if it desired, why is that correct, if not, I belive this can not improve performance
There was a problem hiding this comment.
This part can be reverted as I'm not sure it needs to do multiple iterations.
There was a problem hiding this comment.
But it is really faster than? Please provide microbenchmark which shows count(array) is really slower.
There was a problem hiding this comment.
the problem is not with the complexity O(1) of count(), the point is that referencing to where the array's length is stored tends to be slower than accessing data from a variable. This makes them meaningful in the loop.
There was a problem hiding this comment.
yes, it seems a little bit faster https://3v4l.org/i2dqh
|
|
||
| // transform A & (B & C) to A & B & C | ||
| for ($i = 0; $i < count($types); $i++) { | ||
| for ($i = 0; $i < $typesCount; $i++) { |
There was a problem hiding this comment.
previously it was called in every loop iteration which would make it O(n) then I guess? AFAIK this one of the micro optimizations that also the EA Extended plugin for phpstorm suggests and it definitely makes sense IMO.
|
well i accidentally clicked the wrong ;) |
|
Thank you! |
Idea from #747