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

[Stefan Johansson]

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/

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
>>>>
>>>>
>>>>
>>