RFR 8140685: Fix backtrace building to not rely on constant pool merging

Coleen Phillimore coleen.phillimore at oracle.com
Mon Nov 2 22:03:15 UTC 2015


David thank you for looking at this change.

On 11/1/15 8:12 PM, David Holmes wrote:
> Hi Coleen,
>
> Just a superficial review as I'm not familiar with this code.
>
> On 31/10/2015 8:16 AM, Coleen Phillimore wrote:
>>
>> On 10/30/15 6:01 PM, Coleen Phillimore wrote:
>>> Summary: Save Throwable in a list which is a weak GC root for walking
>>> during MetadataOnStackMark
>
> I'm unclear on terminology here. At the Java level an object that is 
> only weakly reachable can be gc'd. But here we seem to use a "weak" 
> root to prevent something being gc'd - ???
>
>>> This was the original implementation that I had for finding Method* in
>>> backtraces but it was slow because I had used jweak to save the
>>> Throwable object.   Using internal vm weak GC roots, this does not
>>> cause a performance regression in refWorkload.  This fix is more
>>> straightforward and does not rely on the constant pool index of the
>>> merged constant pool during redefinition to find the method's name.
>>> This is one fix that enables removing merged constant pools during
>>> redefinition (work in progress).  It also is would be used as a model
>>> for JEP 259: Stack Walking API
>>> https://bugs.openjdk.java.net/browse/JDK-8043814.
>>>
>>> The code that registers MemberNames for replacing Method* during
>>> redefinition may need to use the same mechanism for performance, if
>>> MemberName arrays are used for the Stack Walking API.
>>>
>>> I assume this will generate comments from the GC group.
>>>
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8140685
>>>
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8140685.01/
>> bug link https://bugs.openjdk.java.net/browse/JDK-8140685
>
> Some minor comments:
>
> src/share/vm/classfile/javaClasses.hpp
>
> Typo:   // Mark methods as that are
>
> src/share/vm/classfile/backtrace.hpp
>
> _backtraces_head should be volatile as you update via CAS.
>
> src/share/vm/classfile/backtrace.cpp
>
> Minor nit: the "exchanged" variable in add should really be called 
> "found".

I fixed these issues.

thanks,
Coleen
>
> Thanks,
> David
> -----
>
>>>
>>> Tested with RBT quick (former "quick" tests), jdk/java/lang/instrument
>>> tests and the test that I wrote for handling backtraces with
>>> redefinition in hotspot/test/runtime/RedefineTests.
>>>
>>> Thanks,
>>> Coleen
>>



More information about the hotspot-dev mailing list