RFR: 8314236: Overflow in Collections.rotate [v2]

[Nikita Sakharin]

On Tue, 15 Aug 2023 09:54:48 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Nikita Sakharin has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8314236: change bug number and summary
>
> src/java.base/share/classes/java/util/Collections.java line 810:
> 
>> 808: 
>> 809:     private static <T> void rotate1(final List<T> list, int distance) {
>> 810:         final int size = list.size();
> 
> Let's keep the style, and do a focused fix: skip adding `final` here. (`final` mostly matters for fields).

Done. In order to be consistent I also removed `final` for `bound` variable.

> src/java.base/share/classes/java/util/Collections.java line 813:
> 
>> 811:         if (size == 0)
>> 812:             return;
>> 813:         distance %= size;
> 
> Same, let's keep the original style.

Done

> test/jdk/java/util/Collections/RotateHuge.java line 27:
> 
>> 25:  * @test
>> 26:  * @bug 8314236
>> 27:  * @summary Overflow in Collections.rotate
> 
> Since this test takes >4G of heap to hold the list with compressed oops, and >8G of heap without compressed oops, we need to run it in a separate VM with enough headroom, something like this:
> 
> 
>  * @test
>  * @bug 8314236
>  * @summary Overflow in Collections.rotate
>  * @requires (sun.arch.data.model == "64" & os.maxMemory >= 16g)
>  * @run main/othervm -Xmx12g RotateHuge

Thanks! Done

> test/jdk/java/util/Collections/RotateHuge.java line 40:
> 
>> 38:         final List<Byte> list = new ArrayList<>(size);
>> 39:         for (int i = 0; i < size; ++i)
>> 40:             list.add((byte) 0);
> 
> We don't actually need to rely on auto-boxing here, right?
> 
> Suggestion:
> 
>         final Object o = new Object();
>         final List<Object> list = new ArrayList<>(size);
>         for (int i = 0; i < size; i++) {
>             list.add(o);
>         }

Done

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15270#discussion_r1294467547
PR Review Comment: https://git.openjdk.org/jdk/pull/15270#discussion_r1294466664
PR Review Comment: https://git.openjdk.org/jdk/pull/15270#discussion_r1294466347
PR Review Comment: https://git.openjdk.org/jdk/pull/15270#discussion_r1294467775