On Fri, 28 Jun 2024 10:22:58 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> 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_UP...
@cl4es provided a better idea, which is to change the branch of switch instead
of using ForceInline. I have another approach:
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 -> true;
default -> c == PERCENT_SIGN;
};
-------------
PR Comment: https://git.openjdk.org/jdk/pull/19926#issuecomment-2196652531
