[11u] RFR 8224202: Speed up Properties.load

[Langer, Christoph]

Hi Aleksey,

> -----Original Message-----
> From: jdk-updates-dev <jdk-updates-dev-bounces at openjdk.java.net> On
> Behalf Of Aleksey Shipilev
> Sent: Mittwoch, 7. August 2019 17:03
> To: jdk-updates-dev at openjdk.java.net
> Subject: [11u] RFR 8224202: Speed up Properties.load
> 
> Original RFE:
>   https://bugs.openjdk.java.net/browse/JDK-8224202
>   http://hg.openjdk.java.net/jdk/jdk/rev/cdb107ca16e6
> 
> Patch applies cleanly to 11u, but does not compile, because there is no
> ArraysSupport.newLength
> (introduced in 13 with JDK-8223593). So, I had to rewrite that block with:
> 
> diff -r 436f75b57855 src/java.base/share/classes/java/util/Properties.java
> --- a/src/java.base/share/classes/java/util/Properties.java     Thu May 23
> 18:43:47 2019 +0200
> +++ b/src/java.base/share/classes/java/util/Properties.java     Wed Aug 07
> 16:09:07 2019 +0200
> @@ -49,5 +49,4 @@
>  import jdk.internal.misc.SharedSecrets;
>  import jdk.internal.misc.Unsafe;
> -import jdk.internal.util.ArraysSupport;
>  import jdk.internal.util.xml.PropertiesDefaultHandler;
> 
> @@ -568,5 +567,13 @@
>                      lineBuf[len++] = c;
>                      if (len == lineBuf.length) {
> -                        lineBuf = new char[ArraysSupport.newLength(len, 1, len)];
> +                        int maxLen = Integer.MAX_VALUE - 8; // VM allocation limit
> +                        int newLen = len*2;
> +                        if (newLen < 0) { // overflow?
> +                            newLen = maxLen;
> +                        }
> +                        if (newLen > maxLen) { // over VM limit?
> +                            newLen = maxLen;
> +                        }
> +                        lineBuf = new char[newLen];
>                          System.arraycopy(this.lineBuf, 0, lineBuf, 0, len);
>                          this.lineBuf = lineBuf;
> 
> It is similar to what we had before, but with VM allocation limit treatment
> that
> ArraysSupport.newLength would give us.

Oh boy, it really took me a while to understand what ArraysSupport::newLength is supposed to do. And actually, after I think I grasped it, I would think that your inline implementation isn't quite correct.

The idea of ArraysSupport.newLength is to provide something that makes the array ideally grow the prefGrowth value, if it fits within MAX_ARRAY_LENGTH (Integer.MAX_VALUE - 8). If it doesn't, then grow to the maximum possible, satisfying the minimal growth and ideally being within the MAX_ARRAY_LENGTH boundary (but try as hard as Integer.MAX_VALUE, if it is really necessary). But in any case, the array needs to grow something or we need to bailout with OOM. And that wouldn't happen in your coding... if you're at maxLen already and a char is added, you will probably get an AIOOBE, since your Array wouldn't grow beyond maxLen. This is probably just a theoretical cornercase, though.

The implementation in the sense of ArraysSupport.newLength would be:

int maxLen = Integer.MAX_VALUE - 8; // highest VM allocation limit where one can still sleep well

// try to double the size...
int newLen = len * 2;

// ...but try to stay out of the crazy range...
if (newLen < 0 || newLen > maxLen) {
    newLen = Integer.MAX_VALUE - 8;
}

// ... and when already desperate then we need to grab for the last possible straw
if (len >= newLen) {
    if (len != Integer.MAX_VALUE) {
        newLen = len + 1;
    } else {
        throw new OutOfMemoryError("Required array length too large");
    }
}

lineBuf = new char[newLen];

Alternatively, why not add ArraysSupport::newLength? E.g. backport JDK-8223593 (although maybe aph wouldn't like it ��)?

Best regards
Christoph