RFR (XL) 8155672: Remove instanceKlassHandles and KlassHandles
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Mon Mar 13 19:34:07 UTC 2017
On 3/13/17 2:31 PM, serguei.spitsyn at oracle.com wrote:
> Hi Coleen,
>
> It looks good.
> Nice cleanup!
>
> http://oklahoma.us.oracle.com/~cphillim/webrev/8155627.src.closed.02/webrev/share/vm/jfr/jfrClassAdapter.cpp.frames.html
>
>
> The HandleMark's at 1743 & 1784 can be not needed anymore.
Thank you for reviewing! I suspect many HandleMarks lying around the
code aren't needed anymore, because KlassHandles (and even
methodHandles) aren't cleaned up using HandleMark. I've removed those
two (and ran some retests because this is getting dangerous).
Are you going to review the open too?
thank you!
Coleen
>
> Thanks,
> Serguei
>
>
> On 3/12/17 21:18, coleen.phillimore at oracle.com wrote:
>>
>> As requested by David on the closed part of this review, I fixed
>> variable names of the form h_ik and klass_h where the 'h' indicated
>> that the klass was in a handle.
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8155672.01/webrev
>>
>> Also, fixing InstanceKlass to not pass this_k can be done in a
>> separate cleanup.
>>
>> Thanks,
>> Coleen
>>
>> On 3/12/17 11:32 AM, coleen.phillimore at oracle.com wrote:
>>> Summary: Use unhandled pointers for Klass and InstanceKlass, remove
>>> handles with no implementation.
>>>
>>> These are fairly extensive changes. The KlassHandle types have been
>>> dummy types since permgen elimination and were thought to be useful
>>> for future features. They aren't, so can be removed (see bug for
>>> more details). This allows stricter typing because you can use the
>>> more specific type, and using const more. I didn't add const to
>>> these changes, because of the cascading effect.
>>>
>>> The change isn't hard to review, just tedious. The main bug that I
>>> had was redeclaring a type inside a scope, and
>>> InstanceKlass::cast(k) can't take a NULL k, whereas
>>> instanceKlassHandle ik(THREAD, k) could.
>>>
>>> It's so nice being able to single step on gdb without going into
>>> KlassHandle constructors!
>>>
>>> open webrev at http://cr.openjdk.java.net/~coleenp/8155672.01/webrev
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8155672
>>>
>>> Tested with all hotspot jtreg tests, java/lang/invoke tests,
>>> java/lang/instrument tests, all closed tonga colocated tests, and JPRT.
>>>
>>> I can continue to hold this change for a convenient time for merging
>>> purposes with other people's JDK10 changes that they've been
>>> holding, or if there are other jdk9 changes that are likely to cause
>>> a problem for merging. I'll update the copyrights to 2017 on commit.
>>>
>>> Thanks,
>>> Coleen
>>
>
More information about the hotspot-dev
mailing list