RFR 8129547 (S): Excess entries in BootstrapMethods with the same (bsm, bsmKind, bsmStaticArgs), but different dynamicArgs

[Maurizio Cimadamore]

Ok, I think I know what you meant now; DynamicMethod is used for two 
different purposes:

* for the NameAndType CP entry associated with an invokedynamic 
instruction (pointed by the CONSTANT_InvokeDynamic entry); this bit 
_requires_ dynamic info to be taken into account - as different invoked 
types need to map to different entries - even if their underlying BSM 
entry is the same.

* as keys in the BootstrapMethod attribute; in this case you want the 
dynamic info to be gone, as all it's going to be stored in the table is 
a bsmKind + static args list.

How about something like this:

http://cr.openjdk.java.net/~mcimadamore/8129547/

Maurizio



On 17/08/15 14:48, Maurizio Cimadamore wrote:
> Hi Alex,
> the only dfference I see between your patch and the current code is 
> that the current code is comparing the invokedType, while yours is not 
> - am I reading your patch correctly?
>
> If that's the case, wouldn't dropping the call to super.equals from 
> Pool.DynamicMethod also resolve the issue?
>
> I'm, saying this because, from a design perspective, javac symbols 
> usually don't care about uniqueness etc. - that's an extra value added 
> when storing them into a constant pool.
>
> Maurizio
>
> On 17/08/15 13:33, Aleksey Shipilev wrote:
>> Hi,
>>
>> This issue gets into way with my current work:
>>    https://bugs.openjdk.java.net/browse/JDK-8129547
>>
>> There is a proof-of-concept patch:
>>    http://cr.openjdk.java.net/~shade/8129547/webrev.00/
>>
>> The crux of an issue seems to be the Method.equals() call in
>> DynamicMethod.equals(), that compares the type. The type of
>> DynamicMethod includes the DynamicMethodSymbol.type, that includes
>> dynamic args. It seems DynamicMethodSymbol is a better fit for
>> BootstrapMethod table key, since it includes BSM symbol and static
>> arguments only.
>>
>> I would be grateful if somebody from compiler team can help me out with
>> this. Is this the fix above valid? Can you do it better?
>>
>> How would one write a regression test for it? My cursory grep through
>> langtools tests does not yield a clear way to emit a special-shaped indy
>> for such a test.
>>
>> Thanks,
>> -Aleksey
>>
>