aherbert commented on PR #139:
URL: https://github.com/apache/commons-numbers/pull/139#issuecomment-1868552071

   I would be wary of this simplification without a performance test.
   
   In the Numbers class the int methods do not use long arithmetic. The long 
methods do not use BigInteger. This is unlike those methods in my JDK 8 source 
code which do and _may_ be slower. A quick check in JDK 21 finds this is still 
not an intrinsic method [1]. 
   
   Note that the Numbers methods are based on the Hacker's Delight book which 
is not free, thus it is not easy to check the
   implementation against the source.
   
   However, there is frequent use of Hacker's Delight in the JDK source. So I 
wonder why they have not use this trick here.
   
   I can create a quick JMH benchmark to test the Numbers methods against the 
JDK. The int method may not be faster as long divide should be supported on 
most hardware. But avoiding BigInteger divide may be noticeable.
   
   Alex
   
   [1] [VM Intrinsics Explorer - HotSpot Intrinsics for 
OpenJDK21](https://chriswhocodes.com/hotspot_intrinsics_openjdk21.html)


-- 
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]

Reply via email to