CMS parallel initial mark

[Thomas Schatzl]

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.

Thomas