RFR: 8316426: Optimization for HexFormat.formatHex

[Chen Liang]

On Fri, 15 Sep 2023 18:04:29 GMT, 温绍锦 <duke at openjdk.org> wrote:

> In the improvement of @cl4es PR #15591, the advantages of non-lookup-table were discussed.
> 
> But if the input is byte[], using lookup table can improve performance.
> 
> For HexFormat#formatHex(Appendable, byte[]) and HexFormat#formatHex(byte[]), If the length of byte[] is larger, the performance of table lookup will be improved more obviously.

Marked as reviewed by liach (Author).

I have created a JBS issue including the 2 confirmed performance improvements in this patch: https://bugs.openjdk.org/browse/JDK-8316426

src/java.base/share/classes/java/util/HexFormat.java line 410:

> 408:         if (length > 0) {
> 409:             try {
> 410:                 String s = formatOptDelimiter(bytes, fromIndex, toIndex);

Great cleanup over here 👍👍👍

src/java.base/share/classes/java/util/HexFormat.java line 422:

> 420:                         toHexDigits(out, bytes[fromIndex + i]);
> 421:                     }
> 422:                     out.append(suffix);

Maybe change this whole `else` block to

                    for (int i = 0; i < length; i++) {
                        if (i > 0)
                            out.append(delimiter);
                        out.append(prefix);
                        toHexDigits(out, bytes[fromIndex + i]);
                        out.append(suffix);
                    }

for clarity?

src/java.base/share/classes/java/util/HexFormat.java line 664:

> 662:     public char toHighHexDigit(int value) {
> 663:         value = (value >> 4) & 0xf;
> 664:         return (char) ((value < 10 ? '0' : hexBase) + value);

These changes seem to be the reason `toHexDigitsLong` consistently sees a slight performance drop. Can any professional engineer explain why the old version, which would be like `(char) (value < 10 ? '0' + value : hexBase + value)`, is slightly faster?

Since the effect of this change `hexBase` is not clear, I recommend dropping this for now (remove the field and changes to the 2 methods).

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

PR Review: https://git.openjdk.org/jdk/pull/15768#pullrequestreview-1631092880
PR Comment: https://git.openjdk.org/jdk/pull/15768#issuecomment-1723515203
PR Review Comment: https://git.openjdk.org/jdk/pull/15768#discussion_r1328749231
PR Review Comment: https://git.openjdk.org/jdk/pull/15768#discussion_r1328748386
PR Review Comment: https://git.openjdk.org/jdk/pull/15768#discussion_r1328763175