[RESUMED] RFR: 8158946 - btree009 fails with assert(s > 0) failed: Bad size calculated

[Derek White]

Hi Thomas,

Thanks for the comments! Questions below...

On 6/22/16 12:10 PM, Thomas Schatzl wrote:
> Hi,
>
> On Tue, 2016-06-21 at 22:07 -0400, Kim Barrett wrote:
>>> On Jun 21, 2016, at 6:36 PM, Thomas Schatzl <thomas.schatzl at oracle.
>>> com> wrote:
>>>
>>> Hi,
>>>
>>> On Tue, 2016-06-21 at 18:27 -0400, Derek White wrote:
>>>> Here's a more complete solution to the problem.
>>>>
>>>> As Kim mentioned, the new version sets the object size field of
>>>> a
>>>> java.lang.Class object before it sets the object's header, so GC
>>>> can
>>>> reliably get the size of any object with it's header set.
>>>    just briefly looking over the code (maybe I am overlooking
>>> something
>>> obvious, I am only looking at the webrev) - wouldn't the code need
>>> some
>>> compiler and memory barriers that make sure that this required
>>> read/write orderings are observed?
>> I agree with Thomas.  I think there needs to be an
>> OrderAccess::storestore() between the setting of the size and the
>> setting of the klass.
> Also the reader thread needs barriers.
>
>> And that makes me wonder why
>> CollectedHeap::post_allocation_setup_array doesn't appear to have
>> a similar store barrier.  It even has the corresponding comment about
>> how the array length must be set before setting the _klass field so
>> the object is parsable by concurrent GC.
>>
>> Maybe this only causes problems when the object is allocated in the
>> old gen (perhaps because it is large).  Is there some other path for
>> large arrays, so we don't have a barrier for every array allocation?
>> I hope I'm missing something...
> I do not know for CMS, but G1's humongous objects do have a storestore
> barrier at the correct place (and it should have the corresponding at
> the reader side). These are the only direct old gen allocations G1 ever
> does.
Where is this barrier used? I thought the header setting was done up at 
CollectedHeap::array_allocate(), outside of G1 code?
> In any case, as soon as CMS uses this method for old gen allocation, it
> needs to have the necessary barriers (obviously).

I think for CMS, reading and writing are protected by the 
cms_space->freelistLock(). For example,
the CMS sweeper holds the freelistLock. The Java thread trying to 
allocate requests, then gets the freelistLock(), and the sweeper 
re-aquires the freelistLock() before resuming the sweep (and reading).

So I'd think that there are plenty of fences for CMS?

  - Derek
>
> It seems that in CMS, the threshold to use a direct old gen allocation
> is pretty high, i.e. object_size > young-gen-capacity, so really really
> seldom (in GenCollectorPolicy::should_try_older_generation_allocation)
> (or if the gc locker is active).
>
>>>> This fix works by adding a CollectedHeap::class_allocate() method
>>>> and
>>>> associated helpers. These are implemented in
>>>> CollectedHeap.inline.hpp
>>>> only because they share so much structure and code with
>>>> CollectedHeap::obj_allocate() that I thought it best to keep
>>>> them
>>>> together (even though there is no performance reason to have the
>>>> new
>>>> code inlined).
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~drwhite/8158946/webrev.02
>>>> jprt in progress...
> Thanks,
>    Thomas
>