Review request: 8013160: NPG: Remove unnecessary mark stack draining after CodeCache::do_unloading

[Stefan Karlsson]

On 4/24/13 10:32 PM, Coleen Phillimore wrote:
>
> Stefan,
>
> Is there a common place for this code?

I don't think there's any common place in the GCs for this. I'm open for 
suggestions if you know a good place in the runtime code.

> It's 4 copies of the similar code, which takes me a while to find when 
> I set out looking for it.

Yes, I tried to make them look alike so that we/I have an opportunity to 
extract it out to one function. The fact that CMS does this differently 
made me hesitant to combine them.

> In the future, I think StringTable::unlink() could be moved after 
> CLDG::purge() which I thought should be around here too (but it's 
> not).   The only reason StringTable::unlink() used to be here was 
> because it used to have Oops in it.

Don't you mean the SymbolTable? I'm OK with moving the SymbolTable, but 
not the StringTable ...

>    Can you unite this duplicated code?

It's easy to unit the code for psMarkSweep, genMarkSweep, g1MarkSweep 
and psParallelCompact but it's a bit more work to also unify the CMS 
code. Would it be good enough for this patch if I united the former GCs? 
And maybe handle CMS when G1 also starts doing class unloading?

thanks,
StefanK
>
> Thanks,
> Coleen
>
> On 4/24/2013 4:18 PM, Stefan Karlsson wrote:
>> http://cr.openjdk.java.net/~stefank/8013160/webrev.00/
>>
>> There's some remnants from the PermGen that's confusing in the class 
>> unloading code. This patch removes the unnecessary code, cleans up 
>> some comments and unifies the code structure over different GCs.
>>
>> From the Bug report:
>> ---
>> The PermGen Removal changed CompiledICHolders from being Java objects 
>> to C++ objects.
>>
>> CodeCache::do_unloading(...) used to take a keep_alive closure that 
>> was applied to CompiledICHolders deep down in nmethod::do_unloading. 
>> Since CompileICHolders don't move anymore, and we have other ways to 
>> keep them alive, we don't use the keep_alive closures in these 
>> functions anymore.
>>
>> Because of this, CodeCache::do_unloading will not populate the 
>> marking stacks, and we can remove the call to drain the stacks after 
>> the calls to CodeCache::do_unloading.
>>
>> This also has the neat effect that we now can verify that the marking 
>> has really completed before we start unloading classes, which makes 
>> the code easier to maintain/understand.
>> ---
>>
>> thanks,
>> StefanK
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20130424/df28c15d/attachment.htm>