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

[Thomas Schatzl]

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.

In any case, as soon as CMS uses this method for old gen allocation, it
needs to have the necessary barriers (obviously).

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