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

[Coleen Phillimore]

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/d6aa8674/attachment.html