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

[Aleksey Shipilev]

Hi Maurizio,

Thanks for an idea! It almost works, but not quite: super.equals() from
Method checks the type, so we need to guard super.*, as you did in
hashCode(). We also need a proper subclass check for the keys alone,
which means we might move equals() there, and make key a proper subclass
to gain access to "other".

In short, this version works:
 http://cr.openjdk.java.net/~shade/8129547/webrev.01/

Thanks,
-Aleksey

On 17.08.2015 17:15, Maurizio Cimadamore wrote:
> 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
>>>
>>
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20150817/7fd6e14e/signature.asc>