On Wed, 13 Sep 2023 01:24:05 GMT, Shaojin Wen <d...@openjdk.org> wrote:

> 1. Reduce duplicate stringSize code
> 2. Move java.lang.StringLatin1.getChars to 
> jdk.internal.util.DecimalDigits::getCharLatin1,not only java.lang, other 
> packages also need to use this method

@cl4es can you help me to review this PR?

private static final MethodHandle PUT_CHAR_DIGIT;

    static {
        try {
            Lookup lookup = MethodHandles.lookup();
            PUT_CHAR_DIGIT = lookup.findStatic(FormatItem.class, "putByte",
                    MethodType.methodType(void.class,
                            byte[].class, int.class, int.class));
        } catch (ReflectiveOperationException ex) {
            throw new AssertionError("putByte lookup failed", ex);
        }
    }

    private static void putByte(byte[] buffer, int index, int ch) {
        buffer[index] = (byte)ch;
    }

   int length = DecimalDigits.INSTANCE.size(value);
   DecimalDigits.INSTANCE.digits(value, this.digits, length, PUT_CHAR_DIGIT);


In the current implementation, FormatItem only calls DecimalDigits using the 
encoding of LATIN1

> Since the current `MethodHandle`-based char putter can only put 1-byte at 
> once, have you considered something like this:
> 
> ```java
> // A replacement for setter MethodHandle, or VarHandle, to accept multiple 
> value types
> public interface DigitConsumer {
>     void putChar(byte[] array, int index, byte value);
>     // put 2 byte-sized chars at once, encoded little endian
>     void putChar2(byte[] array, int index, short value);
>     // you can add putChar4, putChar8, etc. if you need
> }
> ```
> 
> and `StringConcatHelper.selectPutChar` will return a `DigitConsumer` instead 
> of a `MethodHandle`.
> 
> Currently, you are allocating a new byte array for every number in the 
> format, which I deem very inefficient.

Code based on MethodHandler is difficult to maintain and debug. We should 
remove the use of MethodHandle in FormatItem.

There are too many changes that need to be made to reduce the duplication of 
code in the digits method and the getCharsLatin1.

Can we temporarily allow duplicate code for digits and getCharLatin1? Revert 
changes related to FormatItem. After this PR is completed, submit a new PR to 
change FormatItem and reduce duplicate code.

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

PR Comment: https://git.openjdk.org/jdk/pull/15699#issuecomment-1717697947
PR Comment: https://git.openjdk.org/jdk/pull/15699#issuecomment-1731821226
PR Comment: https://git.openjdk.org/jdk/pull/15699#issuecomment-1732271564
PR Comment: https://git.openjdk.org/jdk/pull/15699#issuecomment-1740061373

Reply via email to