On Fri, 15 Sep 2023 18:04:29 GMT, 温绍锦 <d...@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
