On Wed, 11 Sep 2024 15:51:25 GMT, fabioromano1 <d...@openjdk.org> wrote:
>> This implementation of MutableBigInteger.leftShift(int) optimizes the
>> current version, avoiding unnecessary copy of the MutableBigInteger's value
>> content and performing the primitive shifting only in the original portion
>> of the value array rather than in the value yet extended with trailing zeros.
>
> fabioromano1 has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Use supported annotation by jtreg in tests
src/java.base/share/classes/java/math/MutableBigInteger.java line 709:
> 707: * Assumes that intLen > 0, n > 0 for speed
> 708: */
> 709: private final void primitiveRightShift(int n) {
Plz remove `final` from `private` primitive shift methods. (The `final`s were
there long before your PR.)
src/java.base/share/classes/java/math/MutableBigInteger.java line 722:
> 720: * {@code (resPos <= offset || resPos >= offset + intLen)}.
> 721: */
> 722: private final void primitiveRightShift(int n, int[] result, int
> resPos) {
Suggestion:
private final void primitiveRightShift(int n, int[] result, int resOff) {
or something more similar to "offset".
Same for the left shift.
src/java.base/share/classes/java/math/MutableBigInteger.java line 753:
> 751: * {@code (resPos <= offset || resPos >= offset + intLen)}.
> 752: */
> 753: private final void primitiveLeftShift(int n, int[] result, int
> resPos) {
I think this method can be made more symmetrical w.r.t. `primitiveRightShift()`
if starting from the right (least significant `int`).
test/jdk/java/math/BigInteger/MutableBigIntegerShiftTests.java line 49:
> 47:
> 48: static final int ORDER_SMALL = 60;
> 49: static final int ORDER_MEDIUM = 100;
Only now it occurs to me that these can be `private`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20008#discussion_r1756584630
PR Review Comment: https://git.openjdk.org/jdk/pull/20008#discussion_r1756587300
PR Review Comment: https://git.openjdk.org/jdk/pull/20008#discussion_r1756584738
PR Review Comment: https://git.openjdk.org/jdk/pull/20008#discussion_r1756584885
