8165949: Serial and ConcMarkSweep do not unload strings when class unloading is disabled

[Mikael Gerdin]

Hi Stefan

On 2016-09-20 14:55, Stefan Johansson wrote:
> Hi Mikael,
>
> On 2016-09-20 14:24, Mikael Gerdin wrote:
>> Hi,
>>
>> On 2016-09-20 11:34, Stefan Johansson wrote:
>>> Hi all,
>>>
>>> Please review this fix for:
>>> https://bugs.openjdk.java.net/browse/JDK-8165949
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~sjohanss/8165949/hotspot.00/
>>
>> As mentioned in my other review the comment above enum ScanningOption
>> might need an update.
>>
>> I think I'd prefer cms_process_roots instead of conc_process_roots
>> because I think it should be clear that this is cms-specific code.
>>
> Thanks for reviewing, change the name per your request:
> Full: http://cr.openjdk.java.net/~sjohanss/8165949/hotspot.01/
> Inc: http://cr.openjdk.java.net/~sjohanss/8165949/hotspot.00-01/

Looks good.
/Mikael

>
> Thanks,
> Stefan
>
>> Otherwise it looks good!
>> /Mikael
>>
>>
>>>
>>> Summary:
>>> This fix builds on top of the sub-task cleanup out for review in:
>>> 8166276: Refactor gen_process_roots to allow simpler fix for 8165949
>>>
>>> The cleanup task splits gen_process_roots into two version, one for
>>> young and one for old. This fix takes this one step further and split up
>>> the old version to have one version for the full-GC case and one for the
>>> concurrent case used in CMS. This simplifies the methods somewhat and
>>> also allows to do the bug fix very simple.
>>>
>>> The problem we want to solve is for full-GCs when ClassUnloading is
>>> disabled. In this case the old code translated ClassUnloading = false to
>>> mark all oops as live, but this is not true for strings in the string
>>> table. So after the split of old_process_roots into conc_process_roots
>>> and full_process_roots, full_process_roots can use the is_adjust_phase
>>> flag to avoid treating all oops in the marking phase.
>>>
>>> Testing:
>>> * JTREG tests locally
>>> * RBT on hs-gc tier 2-4
>>> * Verified that test/runtime/interned/SanityTest.java works for all GC
>>> with ClassUnloading disabled after the fix.
>>>
>>> Thanks,
>>> Stefan
>>>
>>>
>>>
>