Ivan,

Many thanks for doing this! This code is both error-prone and abundant in the
JDK, which indeed makes it a perfect candidate for refactoring.

Since that particular change touches quite a few foundational parts of the
libraries I felt the urge to make sure the change doesn't introduce any
artifacts. I couldn't find any degradation in amortized performances in the
worst case scenarios. You seem to have carefully preserved all the growth (3/2,
2) factors, and +1's for eventual increase. The fact that the tests have passed
adds a lot of confidence too.

Nevertheless, I would strongly suggest you wait for reviews from the experts in
the areas the change touches. At least for Collections and String. Even though
the core of the change seems to be area-agnostic.

On a separate note. I wonder why all assertion statements were commented out
from ArraysSupport in the first place?

Nits:

1. If `ArraysSupport` is the right place for this functionality, then I think we
should respect its formatting and align methods' arguments into a column:

    public static int newCapacity(int oldCapacity,
                                  int growAtLeastBy,
                                  int preferredGrowBy)

2. A typo:

    * @throws OutOfMemoryError if increasing {@code oldCapacity) by
    *                          {@code growAtLeastBy} overfows.
                                                     ~~~~~^

3. I know that this is not new and has been copied from the old code. However,
I'm not sure I understand the meaning of "unless necessary" here:

    /**
     * The maximum size of array to allocate (unless necessary).

-Pavel

Reply via email to