On Thu, 27 Jun 2024 14:12:36 GMT, Shaojin Wen <d...@openjdk.org> wrote:
>> Currently, the java.util.Formatter$Conversion::isValid method is implemented >> based on switch, which cannot be inlined because codeSize > 325. This >> problem can be avoided by implementing it with ImmutableBitSetPredicate. >> >> use `-XX:+UnlockDiagnosticVMOptions -XX:+PrintInlining` to see the master >> branch: >> >> @ 109 java.util.Formatter$Conversion::isValid (358 bytes) failed to >> inline: hot method too big >> >> >> current version >> >> @ 109 java.util.Formatter$Conversion::isValid (10 bytes) inline (hot) >> @ 4 >> jdk.internal.util.ImmutableBitSetPredicate$SmallImmutableBitSetPredicate::test >> (50 bytes) inline (hot) > > Shaojin Wen has updated the pull request incrementally with one additional > commit since the last revision: > > revert & use `@ForceInline` So the compound issue here is that there's a heuristic which prevents inlining of large methods, and that a tableswitch on inputs spanning from `'%' (37)` to `'x' (120)` become expansive enough (350bytes) to make the JIT enforce that rule. As an alternative to `@ForceInline` I tested splitting uppercase and lowercase chars into two switches. This looks a bit nasty, but actually reduces the size of the method: diff --git a/src/java.base/share/classes/java/util/Formatter.java b/src/java.base/share/classes/java/util/Formatter.java index a7d95ee5780..50419f1ea23 100644 --- a/src/java.base/share/classes/java/util/Formatter.java +++ b/src/java.base/share/classes/java/util/Formatter.java @@ -4947,30 +4947,36 @@ static class Conversion { static final char PERCENT_SIGN = '%'; static boolean isValid(char c) { - return switch (c) { - case BOOLEAN, - BOOLEAN_UPPER, - STRING, - STRING_UPPER, - HASHCODE, - HASHCODE_UPPER, - CHARACTER, - CHARACTER_UPPER, - DECIMAL_INTEGER, - OCTAL_INTEGER, - HEXADECIMAL_INTEGER, - HEXADECIMAL_INTEGER_UPPER, - SCIENTIFIC, - SCIENTIFIC_UPPER, - GENERAL, - GENERAL_UPPER, - DECIMAL_FLOAT, - HEXADECIMAL_FLOAT, - HEXADECIMAL_FLOAT_UPPER, - LINE_SEPARATOR, - PERCENT_SIGN -> true; - default -> false; - }; + if (c >= 'a') { + return switch (c) { + case BOOLEAN, + STRING, + HASHCODE, + CHARACTER, + DECIMAL_INTEGER, + OCTAL_INTEGER, + HEXADECIMAL_INTEGER, + SCIENTIFIC, + GENERAL, + DECIMAL_FLOAT, + HEXADECIMAL_FLOAT, + LINE_SEPARATOR -> true; + default -> false; + }; + } else { + return switch (c) { + case BOOLEAN_UPPER, + STRING_UPPER, + HASHCODE_UPPER, + CHARACTER_UPPER, + HEXADECIMAL_INTEGER_UPPER, + SCIENTIFIC_UPPER, + GENERAL_UPPER, + HEXADECIMAL_FLOAT_UPPER, + PERCENT_SIGN -> true; + default -> false; + }; + } } // Returns true iff the Conversion is applicable to all objects. `javap -v java.lang.Formatter$Conversion` static boolean isValid(char); descriptor: (C)Z flags: (0x0008) ACC_STATIC Code: stack=2, locals=1, args_size=1 0: iload_0 1: bipush 97 3: if_icmplt 122 6: iload_0 7: tableswitch { // 97 to 120 97: 116 98: 116 99: 116 100: 116 101: 116 102: 116 103: 116 104: 116 105: 120 106: 120 107: 120 108: 120 109: 120 110: 116 111: 116 112: 120 113: 120 114: 120 115: 116 116: 120 117: 120 118: 120 119: 120 120: 116 default: 120 } 116: iconst_1 117: goto 121 120: iconst_0 121: ireturn 122: iload_0 123: lookupswitch { // 9 37: 204 65: 204 66: 204 67: 204 69: 204 71: 204 72: 204 83: 204 88: 204 default: 208 } 204: iconst_1 205: goto 209 208: iconst_0 209: ireturn ``` And sees an improvement similar to `@ForceInline` on the micro: ``` Name Cnt Base Error Test Error Unit Change stringIntFormat 15 92,447 ± 2,497 82,912 ± 3,317 ns/op 1,11x (p = 0,000*) * = significant ``` Shrinking the size of the .class file by 121 bytes is a fun bonus. ------------- PR Comment: https://git.openjdk.org/jdk/pull/19926#issuecomment-2196585088