Request for review: 7174978: NPG: Fix bactrace builder for class redefinition

[serguei.spitsyn at oracle.com]

Ok.

Thanks!
Serguei


On 12/12/12 9:15 AM, Coleen Phillimore wrote:
>
> Serguei,
>
> Thanks for reviewing this.  I'm reworking it because in the 
> pathological case, if there are no full GC's, we wouldn't reclaim this 
> backtrace C heap storage.
>
> On 12/11/2012 5:31 PM, serguei.spitsyn at oracle.com wrote:
>> Coleen,
>>
>> It looks good in general.
>> Just some questions below.
>>
>>
>> src/share/vm/classfile/javaClasses.cpp
>>
>> 1339 void java_lang_Throwable::mark_on_stack(oop throwable) {
>>             . . .
>> 1352       if (method == NULL) return;
>>
>>   Would it be more safe to continue instead of return?
>>     1352       if (method == NULL) continue;
>
> This is a short circuit that the function above this also has. The 
> trace_next_offset will be null in this case.  If I change it, I'd want 
> to change them both.
>>
>> src/share/vm/classfile/backtrace.cpp
>>    63 void Backtrace::do_unloading() {
>>
>>   I guess, this can be called at a safepoint only.
>>   Would it make sense to place a comment or an assert?
>
> I added an assert because I assume this.  I'm not sure if CMS gc can 
> unload classes without a safepoint, but this code wants to be run at a 
> safepoint.
>
>> I see you already created a new unit test for this:
>>     java/lang/instrument/RedefineMethodInBacktrace.sh
>>
> I think StefanK was going to add this test after I checked in this fix 
> (or has he already added it?)
>
> thanks,
> Coleen
>
>> Thanks,
>> Serguei
>>
>>
>>
>>
>>
>>
>> On 12/10/12 1:15 PM, Coleen Phillimore wrote:
>>>
>>> I have updated this webrev to include cleanups suggested by John 
>>> Rose for the anonymous class fix.   Please review before I add more 
>>> to this!!
>>>
>>> open webrev at http://cr.openjdk.java.net/~coleenp/7174978_2/
>>> bug link at http://bugs.sun.com/view_bug.do?bug_id=7174978
>>>
>>> Thanks,
>>> Coleen
>>>
>>>
>>> On 12/05/2012 02:23 PM, Coleen Phillimore wrote:
>>>> Summary: Save the set of backtraces to use for on stack method 
>>>> walking for redefine classes.
>>>>
>>>> I also moved metadataOnStackMark class to it's own file because 
>>>> it's not only used for redefine classes.   Some metadata can be 
>>>> individually deallocated (eg. the Method* created in the relocator).
>>>>
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/7174978/
>>>> bug link at http://bugs.sun.com/view_bug.do?bug_id=7174978
>>>>
>>>> Ran test that will be added to the jdk/tests in 
>>>> java/lang/instrument/RedefineMethodInBacktrace.sh (to be checked in 
>>>> separately).
>>>>
>>>> thanks,
>>>> Coleen
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20121212/171f2331/attachment.html