Small perf tweak in strlen loop

[PVince81]

See http://stackoverflow.com/questions/6955913/isset-vs-strlen-a-fast-clear-string-length-calculation

@jknockaert @th3fallen @schiesbn @DeepDiver1975

[scrutinizer-notifier]

A new inspection was created.

[PVince81]

also @nickvergessen @LukasReschke @MorrisJobke

[MorrisJobke]

@PVince81 Is there a use case where this is heavily used? Or is this just a random optimization?

[nickvergessen]

Should help a bit, but the line bellow still calls strlen() :(

[PVince81]

Well, at least it doesn't call it twice.

This is the loop in the encryption app that encrypts block by block. So could help for bigger files.

[PVince81]

Usually the blocks have a specific size, so another possible optimization for the second strlen would be to first see if it's a full block with if (isset($block, 6126)) and only if it is not, then call strlen. This would then only call strlen on the last block.

[nickvergessen]

sounds good

[jknockaert]

Sorry, I can't fully assess the impact of this change.

[ghost]

Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/12390/
🚀 Test PASSed.🚀

[jknockaert]

@PVince81 Am I correct to understand that you substitute a test checking if the first character/byte is set for a test on the length of the string? That should work here. Would such a test also improve performance at L429 ($this->cache === '')?

[PVince81]

Not sure. I just remember that we found out that "strlen" is slow compared to "isset".

It is not too bad, we can also put this in for 8.2 as it's not critical.

[PVince81]

Yes, you are right that we substitute the test for length and do it with a "isset" hack.

[nickvergessen]

Okay, let's do it one after the other: 👍

[jknockaert]

👍

[bantu]

Also move $remainingLength = strlen($data); into the if (!($this->readOnly) && ($resultFseek || $positionInFile === $this->size)) { block, then?

[jknockaert]

@bantu You could do that, but performance will improve only marginally. I can think of only two contexts where the if-statement would evaluate to false and a strlen would be avoided by moving it into it: the case where a write is attempt on a file opened in r-mode (that should not happen), or the case where something is written in r+ mode in the middle of an unseekable encrypted stream, which is not supported by the encryption wrapper (and as far as I know r+ mode is currently not used in owncloud).

[MorrisJobke]

stable8.1 is created -> merge