CMS parallel initial mark

[Jon Masamitsu]

On 6/17/13 2:25 AM, Thomas Schatzl wrote:
> hi,
>
> On Thu, 2013-06-13 at 12:27 -0700, Hiroshi Yamauchi wrote:
>>                  - instead of checking whether _eden_chunk_array is
>>                  NULL to detect
>>                  whether you can sample I would prefer if the same
>>                  predicate as in the
>>                  initialization (gch->supports_inline_contig_alloc())
>>                  were used.
>>                  (I think the change just copied what was done in the
>>                  previous version of
>>                  concurrentMarkSweepGeneration.cpp:4515 has been done)
>>                  
>>                  Maybe create a new predicate for that, used
>>                  everywhere.
>>                  
>>                  (btw, throughout this code there is a lot of checking
>>                  whether some
>>                  NEW_C_HEAP_ARRAY returned NULL or not, and does
>>                  elaborate recovery -
>>                  however by default NEW_C_HEAP_ARRAY fails with an OOM
>>                  anyway... - but
>>                  that is certainly out of scope)
>>          
>>          
>>          Thomas, could you file a CR for fixing that?  Thanks.
>>          
>>          
>> Thomas, by a new predicate, do you mean checking
>>
>>    if (gch->supports_inline_contig_alloc())
>>
>> as opposed to
>>
>>    if (_eden_chunk_array != NULL)
>>
>> assuming that the NEW_C_HEAP_ARRAY succeeded?
>>
> If it did not succeed, the NEW_C_HEAP_ARRAY will exit the VM. This check
> is more because _eden_chunk_array is not initialized with a non-NULL
> pointer in all cases - exactly if supports_inline_contig_alloc() is not
> true, no space will be allocated for it.
>
> That's why such guards are needed.
>
>> You're right that the code checks the null-ness of _eden_chunk_array
>> as CMSCollector::sample_eden() did so.
>>
>> I see that CR 8016276 was filed. Given this, but without knowing the
> 8016276 is about that there is no point in having allocation failure
> handling when the VM exits anyway on allocation failure.
>
>>   details of the CR, it seems OK for this patch to keep the null check
>> against _eden_chunk_array as it is?
>>
> Yes, keep it. Also Jon seems to prefer it that way.

Yes.

Thanks.

Jon

>
> Thomas
>
>