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
Changes requested by liach (Author).
Changes requested by liach (Author).
Do Octal and Hex digits need int versions of getChars and stringSize?
Since the current `MethodHandle`-based char putter can only put 1-byte at once,
have you considered something like this:
// 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.
src/java.base/share/classes/java/util/FormatItem.java line 148:
> 146: int length = DecimalDigits.stringSize(value);
> 147: this.digits = new byte[length];
> 148: DecimalDigits.getCharsLatin1(value, length, this.digits);
Sorry missed this one in last round of review. This is wrong if you mix number
fornat items and chinese segments in a String Template.
src/java.base/share/classes/java/util/FormatItem.java line 248:
> 246: public long prepend(long lengthCoder, byte[] buffer) throws
> Throwable {
> 247: MethodHandle putCharMH = selectPutChar(lengthCoder);
> 248: HexDigits.getCharsLatin1(value, (int)lengthCoder, buffer);
This is wrong if the coder is UTF16
src/java.base/share/classes/java/util/FormatItem.java line 296:
> 294: public long prepend(long lengthCoder, byte[] buffer) throws
> Throwable {
> 295: MethodHandle putCharMH = selectPutChar(lengthCoder);
> 296: OctalDigits.getCharsLatin1(value, (int)lengthCoder, buffer);
Same, wrong for UTF16
src/java.base/share/classes/jdk/internal/util/DecimalDigits.java line 216:
> 214: */
> 215: public static int getCharsLatin1(int i, int index, byte[] buf) {
> 216: // Used by trusted callers. Assumes all necessary bounds checks
> have been done by the caller.
Can you move this into the javadoc, like
<strong>Caller must ensure buf has enough capacity for the value to be
written!</strong>
-------------
PR Review: https://git.openjdk.org/jdk/pull/15699#pullrequestreview-1623691652
PR Review: https://git.openjdk.org/jdk/pull/15699#pullrequestreview-1639167213
PR Comment: https://git.openjdk.org/jdk/pull/15699#issuecomment-1730795452
PR Comment: https://git.openjdk.org/jdk/pull/15699#issuecomment-1732267099
PR Review Comment: https://git.openjdk.org/jdk/pull/15699#discussion_r1334265520
PR Review Comment: https://git.openjdk.org/jdk/pull/15699#discussion_r1333894410
PR Review Comment: https://git.openjdk.org/jdk/pull/15699#discussion_r1333894660
PR Review Comment: https://git.openjdk.org/jdk/pull/15699#discussion_r1323970440
