On Thu, 19 Oct 2023 11:58:26 GMT, Claes Redestad <redes...@openjdk.org> wrote:

>> @cl4es 
>> 
>>> Good, narrows it down to what's going on in `prepend(long, byte[], 
>>> String)`. Might boil down to `System.arraycopy`. This method might not be 
>>> optimized for tiny arrays on all platforms. Specializing for single-char 
>>> case:
>>> 
>>> ```java
>>> diff --git a/src/java.base/share/classes/java/lang/String.java 
>>> b/src/java.base/share/classes/java/lang/String.java
>>> index 9b19d7e2ac1..6eb70925dab 100644
>>> --- a/src/java.base/share/classes/java/lang/String.java
>>> +++ b/src/java.base/share/classes/java/lang/String.java
>>> @@ -4723,7 +4723,11 @@ static void repeatCopyRest(byte[] buffer, int 
>>> offset, int limit, int copied) {
>>>       */
>>>      void getBytes(byte[] dst, int dstBegin, byte coder) {
>>>          if (coder() == coder) {
>>> -            System.arraycopy(value, 0, dst, dstBegin << coder, 
>>> value.length);
>>> +            if (value.length == 1) {
>>> +                dst[(dstBegin << coder)] = value[0];
>>> +            } else {
>>> +                System.arraycopy(value, 0, dst, dstBegin << coder, 
>>> value.length);
>>> +            }
>>>          } else {    // this.coder == LATIN && coder == UTF16
>>>              StringLatin1.inflate(value, 0, dst, dstBegin, value.length);
>>>          }
>>> ```
>>> 
>>> .. seem to help the JIT do the right thing consistently, too:
>>> 
>>> ```
>>> Benchmark                                 Mode  Cnt   Score   Error  Units
>>> BigDecimals.testSmallToEngineeringString  avgt   50  11,757 ± 0,480  ns/op
>>> ```
>> 
>> In addition to #16244, will you submit a PR for this?
>
>> In addition to #16244, will you submit a PR for this?
> 
> Once both #16244 and this has been integrated I want to revisit this. The 
> effect of changing `getBytes(byte[], int, byte)` might have disappeared with 
> #16244 since it better guarantees the JIT will constant fold the prefixes 
> thoroughly. We've observed related issues with System.arraycopy, however, see 
> https://bugs.openjdk.org/browse/JDK-8295496 - so I want to evaluate a few 
> different options here, time allowing.

@cl4es @rgiulietti 

Can I add scale2 method to StringLatin1? Like this:


package java.lang;

class StringLatin1 {
    static String scale2(long value) {
        long valueAbs = Math.abs(value);
        long highInt = valueAbs / 100;
        byte[] buf = new byte[Long.stringSize(highInt) + (value < 0 ? 4 : 3)];
        int p = buf.length - 2;
        writeDigitPair(buf, p, (int) (valueAbs % 100));
        buf[--p] = '.';
        getChars(highInt, p, buf);
        if (value < 0) {
            buf[0] = '-';
        }
        if (String.COMPACT_STRINGS) {
            return new String(buf, LATIN1);
        }
        return new String(inflate(buf, 0, buf.length), UTF16);
    }
}

class System {
        private static void setJavaLangAccess() {
                SharedSecrets.setJavaLangAccess(new JavaLangAccess() {
                    public String scale2(long i) {
                        return StringLatin1.scale2(i);
                    }
                }
        }
}



I think this is more readable than using MethodHandle.

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

PR Comment: https://git.openjdk.org/jdk/pull/16006#issuecomment-1775022048

Reply via email to