RFR 8223593 : Refactor code for reallocating storage

Ivan Gerasimov ivan.gerasimov at oracle.com
Fri May 10 23:05:52 UTC 2019


Thank you Pavel for the investigation!

I've seen some of these places, and decided not to touch them with this 
refactoring for various reasons:

java.lang.AbstractStringBuilder - the case has additional complication 
due to interference of coder.  It may make sense to refactor it later 
within a separate enhancement.

java.io.InputStream - do you mean readNBytes()?  It uses different 
strategy: instead of reallocating an array of greater size it maintains 
a list of chunks that are combined on a final step.

java.util.ArrayDeque, java.util.concurrent.* - as Roger has mentioned, 
these belong to JSR-166 project.

java.util.Hashtable - It could actually be refactored with the code like 
this:
+        int newCapacity = ArraysSupport.newCapacity(oldCapacity, 0, 
oldCapacity + 1);
+        if (newCapacity == oldCapacity) {
+            // Keep running with MAX_ARRAY_SIZE buckets
+            return;
+        }

Though, I don't think it's worth doing this because it would separate 
logic around MAX_ARRAY_SIZE across different files.

java.util.regex.Pattern.quote - It also could be refactored as
+    int lenHint = ArraysSupport.newCapacity(s.length(), 0, s.length());

I'm not certain it would add much clarity to the code, so let's leave it 
aside for now.

java.io.BufferedInputStream and java.nio.file.Files - Oh, I missed these 
two.  Thank you for pointing me to them!

I'll share the updated webrev shortly.

With kind regards,
Ivan


On 5/10/19 6:22 AM, Pavel Rappo wrote:
> Ivan,
>
> There seem to be more places that use a somewhat similar pattern. I was
> wondering if you have seen them and decided not to include them in your patch
> for some reason (e.g. they really are quite different)?
>
> Here are some of them:
>
>      java.io.BufferedInputStream
>      java.io.InputStream
>      java.lang.AbstractStringBuilder
>      java.nio.file.Files
>      java.util.ArrayDeque
>      java.util.Hashtable
>      java.util.concurrent.ConcurrentHashMap
>      java.util.concurrent.PriorityBlockingQueue
>      java.util.regex.Pattern.quote
>
> There is also some number of occurrences in the jdk.* modules.
>
>> On 9 May 2019, at 02:50, Ivan Gerasimov <ivan.gerasimov at oracle.com> wrote:
>>
>> Hello!
>>
>> Jdk has several places with similar logic:  an array needs to be reallocated (by at least some defined amount), taking into account the maximum allowed size of arrays.
>>
>> There's clearly an opportunity for refactoring, so it is proposed to introduce a dedicated utility method for calculating the best new size of an array.
>>
>> Would you please help review this enhancement?
>>
>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8223593
>> WEBREV: http://cr.openjdk.java.net/~igerasim/8223593/00/webrev/
>>
>> Mach5 job ran fine.
>>
>> Thanks in advance!
>>
>> -- 
>> With kind regards,
>> Ivan Gerasimov
>>
>

-- 
With kind regards,
Ivan Gerasimov



More information about the core-libs-dev mailing list