Looks good!
/Jesper
Mikael Gerdin skrev den 23/6/15 13:47:
> Hi Poonam,
>> On 2015-06-22 23:19, Poonam Bajaj Parhar wrote:
>> Hello Mikael,
>>>> Thanks for taking a look at the changes.
>>>> Yes, you are right, the best place to add the SO_AllCodeCache
>> root_scanning option is
>> CMSCollector::setup_cms_unloading_and_verification_state(). There the
>> issue is that
>> we are adding/removing the root scanning options based on if the
>> verification options
>> are enabled/disabled.
>>>> SO_AllCodeCache should get added to the root_scanning_options if we are
>> not unloading
>> classes regardless whether we are verifying or not.
>>>> I compared this code with the code of jdk7 and the only part that has
>> changed in there is related
>> to the Perm Gen. I could also reproduce the issue with 7u fastdebug
>> build with
>> -XX:+CMSParallelInitialMarkEnabled and -XX:-VerifyBeforeExit.
>>>> Here's the updated webrev:
>>http://cr.openjdk.java.net/~poonam/8129108/webrev.01/>>>> Failing testcase works fine with these changes.
>> I think the change seems ok, but I'd feel a lot safer if you had some more
> testing results.
>> /Mikael
>>>>> Thanks,
>> Poonam
>>>> On 6/22/2015 6:41 AM, Mikael Gerdin wrote:
>>> Poonam,
>>>>>> On 2015-06-20 06:32, Poonam Bajaj Parhar wrote:
>>>> Hello,
>>>>>>>> Please review this change for bug:
>>>> JDK-8129108: <https://bugs.openjdk.java.net/browse/JDK-8129108> nmethod
>>>> related crash in CM
>>>>>>>> There is an assertion failure with fastdebug build with test
>>>> /nsk.coverage.arguments.arguments002.arguments002/.
>>>> This failure occurs when CMSCollector::_should_unload_classes is false
>>>> and the _roots_scanning_options does
>>>> not contain SO_AllCodeCache:
>>>>>>>> # Internal Error
>>>> (hotspot/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp:4472),
>>>> pid=16267, tid=0xf36ffb70
>>>> # assert(_collector->should_unload_classes() ||
>>>> (_collector->CMSCollector::roots_scanning_options() &
>>>> GenCollectedHeap::SO_AllCodeCache)) failed: if we didn't scan the code
>>>> cache, we have to be ready to drop nmethods with expired weak oops
>>>>>>>>>>>> This problem was exposed with the fix ofJDK-8085965:
>>>> <https://bugs.openjdk.java.net/browse/JDK-8085965> VM hangs in
>>>> C2Compiler,
>>>> that disables CMSClassUnloadingEnabled when -Xnoclassgc or
>>>> -XX:-ClassUnloading
>>>> are specified on the command line.
>>>>>>>> CMSCollector::_roots_scanning_options option should contain
>>>> SO_AllCodeCache to ensure
>>>> that we scan code cache when we are not unloading classes.
>>>>>>>> Webrev: http://cr.openjdk.java.net/~poonam/8129108/webrev/>>>>>> This looks very similar to the code in
>>> CMSCollector::setup_cms_unloading_and_verification_state
>>> which is called from CMSCollector::checkpointRootsInitialWork
>>>>>> I think that the if-clause in setup_cms_unloading... needs to be fixed
>>> to handle the class unloading case separately from the verification
>>> state.
>>> It's not entirely clear to me exactly which cases the complicated
>>> conditions in the if statements try to capture, though. You may want
>>> to look at a previous version of the sources (before perm gen removal)
>>> to better understand the reasoning behind it.
>>>>>> /Mikael
>>>>>>> Testing: JPRT, testcase
>>>> /nsk.coverage.arguments.arguments002.arguments002/
>>>>>>>> Thanks,
>>>> Poonam
>>>>>>