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