RFR: JDK-8237528: Inefficient compilation of Pattern Matching for instanceof

[forax at univ-mlv.fr]

----- Mail original -----
> De: "jan lahoda" <jan.lahoda at oracle.com>
> À: "Remi Forax" <forax at univ-mlv.fr>
> Cc: "compiler-dev" <compiler-dev at openjdk.java.net>
> Envoyé: Vendredi 24 Janvier 2020 09:45:43
> Objet: Re: RFR: JDK-8237528: Inefficient compilation of Pattern Matching for instanceof

> On 23. 01. 20 18:24, Remi Forax wrote:
>> 
>> 
>> ----- Mail original -----
>>> De: "jan lahoda" <jan.lahoda at oracle.com>
>>> À: "compiler-dev" <compiler-dev at openjdk.java.net>
>>> Envoyé: Jeudi 23 Janvier 2020 18:10:36
>>> Objet: RFR: JDK-8237528: Inefficient compilation of Pattern Matching for
>>> instanceof
>> 
>>> Hi,
>>>
>>> When javac compiles:
>>> o instanceof String s
>>>
>>> it desugars this to:
>>>
>>> (let /*synthetic*/ final Object s$temp = o in s$temp instanceof String
>>> && (s = (String)s$temp) == (String)s$temp)
>>>
>>> The "(s = (String)s$temp) == (String)s$temp)" is there to set the
>>> variable "s" to the correct, and still return/have a boolean value. But
>>> this produces several unnecessary instructions for the comparison -
>>> second cast and unnecessary test/jump. This can be changed to:
>>>
>>> (let /*synthetic*/ final Object s$temp = o in s$temp instanceof String
>>> && (let s = (String)s$temp; in true))
>>>
>>> I.e. the (s = (String)s$temp) == (String)s$temp) is replaced with
>>>
>>> (let s = (String)s$temp; in true)
>>>
>>> This is a let expression that will execute the assignment as a statement
>>> and then return true without any tests. This eliminates the unnecessary
>>> cast and test/jump, producing a smaller code (see bytecode before and
>>> after below).
>>>
>> 
>> Nice !
>> You can shave two more bytes (the first two instructions) by detecting that the
>> left part of instanceof is already a local variable, so you don't need the
>> first let.
> 
> Right, I was thinking of that, but inclined to not do that, as it is a
> little tricky (esp. to test) and we don't do it on other places (like in
> switch over string), so it might be a more general effort.

Ok.
yes, removing the let when not needed can be done in several places at once.

BTW, switch over something is not where we should search for clue because as you now, different kinds of switch behave differently from the bytecode POV,
i stumble on this example recently, an empty switch over a String use requireNonNull instead of hashCode while an empty switch over an enum use ordinal.

It's not a big deal until you introduce jep 358 that leaks how null check are done at bytecode level.

> 
> Thanks,
>     Jan

Rémi

> 
>> 
>>> Proposed patch:
>>> http://cr.openjdk.java.net/~jlahoda/8237528/webrev.00/index.html
>>>
>>> JBS:
>>> https://bugs.openjdk.java.net/browse/JDK-8237528
>>>
>>> How does this look?
>>>
>> 
>> Rémi
>> 
> >> Jan