Performance: remove unnecessary repeated function call

[jrfnl]

Why?

The length of the $existing_class does not change within the loop, so it is completely redundant and inefficient to recalculate the length for every loop.

Along the same lines, the count of $this->stack will not change within the loop.

How?

By making the function call once and saving the return value to a variable, we can avoid the unnecessary repeated function calls.

[dmsnell]

hello again @jrfnl - did you measure any results with these changes or confirm that they make things faster or more efficient?

strlen() boils down to a property lookup, not a string-length computation, so the performance should be on par with storing it inside a variable. further, I would like to see any data suggesting that it's possible to measure the impact of this change; my own tests suggest otherwise.

for the case of the stack count in the block parser this patch introduces a breakage because $this->stack does change during every iteration of the while loop it's in. we need to confirm that a small refactor, even one that looks harmless, doesn't introduce a new behavior or defect into the code, and in this case all we need to do is look inside $this->add_block_from_stack(); to see it, as its first line mutates the stack.

[jrfnl]

hello again @jrfnl - did you measure any results with these changes or confirm that they make things faster or more efficient?

strlen() boils down to a property lookup, not a string-length computation, so the performance should be on par with storing it inside a variable. further, I would like to see any data suggesting that it's possible to measure the impact of this change; my own tests suggest otherwise.

I did not do any performance testing, but any kind of function call within a loop condition is generally speaking bad practice, no matter how small the impact may seem. It is also teaching devs bad code patterns.

for the case of the stack count in the block parser this patch introduces a breakage because $this->stack does change during every iteration of the while loop it's in. we need to confirm that a small refactor, even one that looks harmless, doesn't introduce a new behavior or defect into the code, and in this case all we need to do is look inside $this->add_block_from_stack(); to see it, as its first line mutates the stack.

Okay, so clearly I missed that, which does say something about how the code obfuscates what's going on, but that's another matter. I will remove that change from this PR.

[anton-vlasenko]

Thanks for the PR, @jrfnl.
LGTM!

[anton-vlasenko]

In my opinion, calling strlen() with each loop doesn't make sense, as the length of the string doesn't change. So I agree with @jrfnl on this.

[dmsnell]

this is fine, and I'm comfortable merging, but we should note that this doesn't have a clear performance impact.

it is completely redundant and inefficient to recalculate the length for every loop.

this is inaccurate, as it's not completely inefficient and the length is not recomputed - it's returned as a property lookup on the ZSTR value inside PHP.

---

thanks for the patch!

[anton-vlasenko]

@dmsnell If you are comfortable merging, would you please approve this PR?
Unfortunately, I cannot merge it because GH says there are requested changes that have to be addressed.
Many thanks!