> On 4 Apr 2020, at 22:35, Gilles Sadowski <[email protected]> wrote:
>
> Hello.
>
> Le sam. 4 avr. 2020 à 23:16, <[email protected]
> <mailto:[email protected]>> a écrit :
>>
>> This is an automated email from the ASF dual-hosted git repository.
>>
>> aherbert pushed a commit to branch master
>> in repository https://gitbox.apache.org/repos/asf/commons-numbers.git
>>
>> commit 08edae944e4abe7ec4b95a72d3791a9f39fb53c8
>> Author: Alex Herbert <[email protected]>
>> AuthorDate: Sat Apr 4 21:23:08 2020 +0100
>>
>> Remove a branch from equals tests with NaN.
>>
>> A bitwise OR of the booleans is true if either boolean is true. Since
>> they are both already computed then the || operator short circuit (if
>> first is true) cannot avoid having to compute the second boolean.
>>
>> This change converts 3 conditional evaluations to 2 conditional
>> evaluations.
>
> Is this measurable? Is the difference significant?
> I'm not actually asking any benchmarking, but such changes
> should be balanced with the resulting code reduced (IMO)
> readability.
I do not know. I ensured the unit tests had full coverage and then looked at a
few ways to improve the class. This change may be reverted. It may be something
that is done by a good compiler anyway.
There is another change that will eliminate the unreachable code branch. But I
did not make that change as it was less readable IMO:
Original:
return isEqual && !Double.isNaN(x) && !Double.isNaN(y);
Verses:
return isEqual && !Double.isNaN(x + y);
As is this perhaps:
if (((xInt ^ yInt) & SGN_MASK) == 0L) {
Verses:
if (((xInt ^ yInt) >= 0L) {
Given that none of this is in anyway expected to be in performance critical
code I can put it back to the previous version if you prefer.
>
>
> Best,
> Gilles
>
>> ---
>> .../main/java/org/apache/commons/numbers/core/Precision.java | 12
>> ++++++++----
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git
>> a/commons-numbers-core/src/main/java/org/apache/commons/numbers/core/Precision.java
>>
>> b/commons-numbers-core/src/main/java/org/apache/commons/numbers/core/Precision.java
>> index d60806d..4155b51 100644
>> ---
>> a/commons-numbers-core/src/main/java/org/apache/commons/numbers/core/Precision.java
>> +++
>> b/commons-numbers-core/src/main/java/org/apache/commons/numbers/core/Precision.java
>> @@ -177,7 +177,8 @@ public final class Precision {
>> public static boolean equalsIncludingNaN(float x, float y) {
>> final boolean xIsNan = Float.isNaN(x);
>> final boolean yIsNan = Float.isNaN(y);
>> - return xIsNan || yIsNan ?
>> + // Combine the booleans with bitwise OR
>> + return (xIsNan | yIsNan) ?
>> !(xIsNan ^ yIsNan) :
>> equals(x, y, 1);
>> }
>> @@ -274,7 +275,8 @@ public final class Precision {
>> public static boolean equalsIncludingNaN(float x, float y, int maxUlps) {
>> final boolean xIsNan = Float.isNaN(x);
>> final boolean yIsNan = Float.isNaN(y);
>> - return xIsNan || yIsNan ?
>> + // Combine the booleans with bitwise OR
>> + return (xIsNan | yIsNan) ?
>> !(xIsNan ^ yIsNan) :
>> equals(x, y, maxUlps);
>> }
>> @@ -302,7 +304,8 @@ public final class Precision {
>> public static boolean equalsIncludingNaN(double x, double y) {
>> final boolean xIsNan = Double.isNaN(x);
>> final boolean yIsNan = Double.isNaN(y);
>> - return xIsNan || yIsNan ?
>> + // Combine the booleans with bitwise OR
>> + return (xIsNan | yIsNan) ?
>> !(xIsNan ^ yIsNan) :
>> equals(x, y, 1);
>> }
>> @@ -428,7 +431,8 @@ public final class Precision {
>> public static boolean equalsIncludingNaN(double x, double y, int
>> maxUlps) {
>> final boolean xIsNan = Double.isNaN(x);
>> final boolean yIsNan = Double.isNaN(y);
>> - return xIsNan || yIsNan ?
>> + // Combine the booleans with bitwise OR
>> + return (xIsNan | yIsNan) ?
>> !(xIsNan ^ yIsNan) :
>> equals(x, y, maxUlps);
>> }
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> <mailto:[email protected]>
> For additional commands, e-mail: [email protected]
> <mailto:[email protected]>
