RFR 8152271: MemberNameTable doesn't purge stale entries

[Vladimir Ivanov]

Coleen,

Latest version looks good. Thanks for adding clarifying comment in the code.

>>> Summary: Intern MemberNames in table instead of allocating new entries
>>>
>>> For degenerate case, we were leaking native code in the member name
>>> table.  Going with the suggested workaround, it was only a few more
>>> lines of code to intern MemberName and return the MemberName that was
>>> already in the table.
>>
>> I guess I'm very surprised that these were not already being "cached"
>> at some level (ie the Java level where all other reflection-like
>> objects get cached)! I'm not familiar with the mechanics or j.l.invoke
>> so was wondering where MemberName instances actually get created -
>> because I'd like to understand how you get two distinct MemberName
>> instances that compare equal in the first place?
>
> Hi David,
>
> The MemberNames were not being cached. There was a change to attempt to
> cache them in the jdk https://bugs.openjdk.java.net/browse/JDK-8013267
> but it was a much bigger change and we still need access to MemberName
> in the jvm because we have to replace the Method* in them with
> redefinition.
>
> MemberNames get created mostly when you create a MethodHandle.  The
> MemberName is the jvm representation of the member of the class. It's
> not simply a Method* because it contains flags based on how it's
> resolved at linktime.
>
> In the test case provided,
>       MethodHandle mh = lookup.findSpecial(Leak.class, "callMe", mt,
> Leak.class);
>
> this looks up the same method and creates the same MemberName over and
> over.

j.l.i.MemberName implements Cloneable and MN cloning is pervasively used 
during DirectMethodHandle construction (see MN.resolve() and other MN 
instance methods).

Best regards,
Vladimir Ivanov

>>
>> Otherwise changes seem fine - though perhaps
>> MemberNameTable::add_member_name should assert the member_name is not
>> already present, just to confirm those intern==false cases are working
>> as intended.
>
> I want to keep add_member_name simple and not go through the list even
> under ASSERT.  It's okay if some member_name are already present, and
> for the case with JVM_Clone, we want an additional MemberName in the
> table (and that uses add_member_name()).
>
> Thanks for looking at the code.
> Coleen
>
>>
>> Thanks,
>> David
>> -----
>>
>>> This has been performance tested to show no regression and works really
>>> well for the degenerate test case, even though the real percentage of
>>> reused MemberName seems quite small.
>>>
>>> Tested with all runtime nightly tests, tests in
>>> jdk/test/java/lang/invoke.
>>>
>>> open webrev at http://cr.openjdk.java.net/~coleenp/8152271.01/webrev
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8152271
>>>
>>> Thanks,
>>> Coleen
>