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

[Mikael Gerdin]

Hi Stefan,

On 2016-09-21 15:30, Stefan Johansson wrote:
> ThanksMikael,
>
> Got some offline comments from Stefan K and updated the change with
> regard to those comments:
> Full: http://cr.openjdk.java.net/~sjohanss/8165949/hotspot.02/
> Inc: http://cr.openjdk.java.net/~sjohanss/8165949/hotspot.01-02/

Looks good.
/Mikael

>
> The main thing that has changed is that the string table processing has
> moved out to its own function, similar to what was done in G1 root
> processing a few weeks back. This allows us to keep setting up the
> weak_roots_closure the same way as before, and thus make use of an
> optimization in SystemDictionary processing which was lost in my
> previous patch.
>
> Thanks,
> Stefan
>
> On 2016-09-20 15:12, Mikael Gerdin wrote:
>> 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
>>>>>
>>>>>
>>>>>
>>>
>