[performance] Optimize minOf/maxOf methods.

[gnutix]

Hello there,

We're using this library quite extensively in our codebase, and often happen to call methods a lot (like 4 million times in a request), which allows us to witness a few performance bottlenecks. So here's a very simple one we thought we could address.

Creating a new instance of LocalDate/LocalDateTime/LocalTime using the min() and max() functions is unnecessary, and we can achieve the same behavior by using array_shift (extracting the first element of the array to have a reference point for further comparisons). And we don't need to take care of the "empty array" scenario, as there's already a condition above it that throws an Exception. Finally, cherry-on-the-cake, it makes one less loop iteration.

Credit goes to @BastienClement.

[gnutix]

I'm not sure I understand how the code coverage can get lower by replacing a line by another, without conditions ?

[BenMorel]

Hey,

We're using this library quite extensively in our codebase, and often happen to call methods a lot (like 4 million times in a request), which allows us to witness a few performance bottlenecks.

Wow, glad to see how this library is used in the wild!

Creating a new instance of LocalDate/LocalDateTime/LocalTime using the min() and max() functions is unnecessary, and we can achieve the same behavior by using array_shift

Looks neat! I benchmarked it on LocalDate and it shaves off ~24% of the execution time.

I tried it against what I thought would be an even faster version (no array_shift()):

        $min = null;

        foreach ($dates as $date) {
            if ($min === null || $date->isBefore($min)) {
                $min = $date;
            }
        }

But it does not show any significant improvement, so yours is fine.

I'm not sure I understand how the code coverage can get lower by replacing a line by another, without conditions ?

It looks like the line $min = $date; is not executed as the first item is already the minimum value, so the if ($date->isBefore($min)) condition is never triggered. Reordering the elements or adding a new one smaller than the first one should make coverage happy.

[gnutix]

The null version is fine as well indeed. Fixes the lowered code coverage too, so let's go with this one. :)

[BenMorel]

Thanks!