RFR: 8316150: Refactor get chars and string size
Chen Liang
liach at openjdk.org
Mon Oct 16 16:18:01 UTC 2023
On Wed, 13 Sep 2023 01:24:05 GMT, Shaojin Wen <duke at 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
More information about the core-libs-dev
mailing list