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
