aherbert commented on PR #142: URL: https://github.com/apache/commons-numbers/pull/142#issuecomment-2332475910
TLDR; I don't think this will make a difference. The code as is favours the majority case of comparing finite values with an optimisation for double comparisons to skip a NaN check. --- In checking for NaN early you "slow down" the case where two non-NaN values are not equal. We should not really require ULP precision when comparing NaN. It is not expected to be calling this method with NaNs. Note "slow down" may not be measurable if branch prediction skips over the NaN check. Given that the major case is input of non-NaN numbers, then the current code is favouring that. If the values are within precision, then the NaN check is a check for the edge case usage. If they are not within precision then we can skip the NaN check. Note that the double version is slightly different from the float version. It does not require a NaN check when the raw bits have opposite signs. I changed this in commit: 286932d1306606e927c417409c14bb49751c61f8 for [NUMBERS-184](https://issues.apache.org/jira/browse/NUMBERS-184). Unfortunately the assumption for NaNs in doubles does not transfer across to the floats as in that case we would require the ulp argument to be a short (and any NaN value will have an integer representation with the sign bit removed above any possible short). So the current code favours finite arguments that are not equal; your modification favours one or both arguments as NaN. In practice branch prediction should make the correct choice to ignore a NaN possibility and any changes will be very difficult to detect in execution time. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
