[Forwardport] Performance: remove count() form the condition section of a loop

[dimonovp]

Original Pull Request

#13173

Description

In this PR I removed the count() method from condition section for some loops. When this method is used in the loop condition it will be executed every iteration which is not so good for process time (performance).

Fixed Issues (if relevant)

N/A

Manual testing scenarios

N/A

Contribution checklist

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds on Travis CI are green)

[orlangur]

@coderimus @ihor-sviziev @ishakhsuvarov @miguelbalparda hi guys, unfortunately count is O(1) so this change does not have any performance impact while introducing unnecessary variable. See #14079 (comment) for more details.

Can we revert original PR or mark it as not needed for forwardporting maybe instead of porting it to 2.3-develop?

[coderimus]

@orlangur thank you for your reply! I have just one question: All of the above changes are working with count 0()1? Or just this one: b4eede9

Also I think if we will ommit creating $count variable we will call count 2 times: 1st one when check that $count is greater than - and second when will use it in foreach. Maybe in this case it is good to have one variable and do not call method twice?

[orlangur]

Hi @coderimus, all of them. count call execution time does not depend on array size.

Just that there is no need to "not call method twice" from performance perspective. The only overhead is one function call and it is negligible compared to a loop body. It would make some difference only if we did like 100k-1M iterations with a tiny loop body.

[ihor-sviziev]

Hi @orlangur @miguelbalparda , actually we will not have there big changes in performance, because there will not be thousands of iterations, but keeping it different in 2.2-develop and 2.3-develop - not a good idea, because cherry-pick will not be able.
My point - if it was accepted to 2.2-develop, it has very-very small performance improvement and it don't have any regression - let's add it to 2.3-develop in order to have the same code base in those branches.

[magento-engcom-team]

Hi @miguelbalparda, thank you for the review.
ENGCOM-1020 has been created to process this Pull Request

[orlangur]

@ihor-sviziev, thanks for the input

Can we revert original PR or mark it as not needed for forwardporting maybe instead of porting it to 2.3-develop?

That's another option which I raised initially. Personally I'm not a big fan of having two lines of code where we had only one before with the same result.

@coderimus assuming both versions of code perform equally fast, which one would you prefer from readability perspective?