RFR: 8336274: MutableBigInteger.leftShift(int) optimization [v9]

Raffaello Giulietti rgiulietti at openjdk.org
Thu Sep 12 12:05:11 UTC 2024


On Thu, 12 Sep 2024 11:23:24 GMT, fabioromano1 <duke at openjdk.org> wrote:

>> 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.
>
> What about `resFrom`, like the old `MBI.copyAndshift()` method?

Up to you, the current `resPos` is OK as well.

>> 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`).
>
> Yes, it could, but the problem is that in this way the precondition `(resPos <= offset || resPos >= offset + intLen)` would be no longer correct, and in particular `resPos <= offset` is used by `MBI.leftShift()` if `result == value`.

I mean something like

    private void primitiveRightShift(int n, int[] result, int resPos) {
        int[] val = value;
        int n2 = 32 - n;
        int c = 0;
        for (int i = 0; i < intLen; i++) {
            int b = val[offset + i];
            result[resPos + i] = c << n2 | b >>> n;
            c = b;
        }
    }

and

    private void primitiveLeftShift(int n, int[] result, int resPos) {
        int[] val = value;
        int n2 = 32 - n;
        int c = 0;
        for (int i = intLen - 1; i >= 0; i--) {
            int b = val[offset + i];
            result[resPos + i] = c >>> n2 | b << n;
            c = b;
        }

They are compatible with the precondition, right?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20008#discussion_r1756722886
PR Review Comment: https://git.openjdk.org/jdk/pull/20008#discussion_r1756721676


More information about the core-libs-dev mailing list