On Wed, 7 May 2025 15:38:41 GMT, Raffaello Giulietti <[email protected]>
wrote:
>> fabioromano1 has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Removed needless brackets
>
> src/java.base/share/classes/java/math/BigInteger.java line 2643:
>
>> 2641: : new BigInteger(result,
>> newSign).shiftLeft(bitsToShift);
>> 2642: } else {
>> 2643: if ((bitLength() - 1L) * exponent >= (long) MAX_MAG_LENGTH
>> << 5) {
>
> Suggestion:
>
> if ((bitLength() - 1L) * exponent >= 32L * MAX_MAG_LENGTH) {
>
> or
> Suggestion:
>
> if ((bitLength() - 1L) * exponent >= (long) Integer.SIZE *
> MAX_MAG_LENGTH) {
>
>
> Both variant are easier to read, more honest, and exactly as efficient as
> with the shift. The right-hand sides are compile-time constants, so they have
> no impact on runtime performance.
>
> More generally, the runtime compilers are perfectly capable to optimize
> multiplications by constant powers of 2 and replace them with shifts, even if
> the other operand is not a constant.
@rgiulietti What about `(bitLength() - 1L) * exponent >= Integer.MAX_VALUE`?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24690#discussion_r2078107104
