RFR: 8172157: Tighten up contract between concurrent GCs and runtime regarding object allocation and header initialization

[Kim Barrett]

> On Jan 4, 2017, at 6:02 PM, White, Derek <Derek.White at cavium.com> wrote:
> 
> [resending to hotspot-dev]
> 
> There are three issues that came up in the review comments for 8171449 that I tried to handle:
> 1) Inline allocation by jitted code and the interpreter only allocate out of the young gen (either TLABS or eden), and the concurrent GC threads will never scan object in the young gen, so inline-alloctors don't need to ensure memory ordering for the concurrent GC's use. Added comments and assertions.
>  
> 2) Non-inline allocation in the old gen by a concurrent GC should first set an object’s klass field to NULL before setting it to a correct value. Added comments and assertions.
> 3) There should be no race condition between the steps in 2 that allows a concurrent GC thread to see an incorrect value. This is harder to prove, and is not handled in this fix.
>  
> I'm trying to pull some of the constraints out from the depths of the collectors to something closer to the GC/runtime interface, so the non-GC code (writers) can reason about it.
>  
> Bug: https://bugs.openjdk.java.net/browse/JDK-8172157 “Tighten up contract between concurrent GCs and runtime regarding object allocation and header initialization”
>  
> Webrev: http://cr.openjdk.java.net/~drwhite/8172157/webrev.02/
>  
> Thanks,
>  
> 	• Derek

Note that I think this sort of thing is an enhancement, rather than a
bug as the JIRA issue is presently categorized.  Nothing is broken;
it's just harder to understand and modify the code than we might
prefer.  As such, I think work along this line ought to be deferred
out of JDK 9.

------------------------------------------------------------------------------ 
src/share/vm/gc/shared/collectedHeap.hpp
 132   // Concurrent collectors must never allocate a TLAB from the old generation,
 133   // because inline-allocation in TLABS by jitted code does not enforce
 134   // memory ordering consistency for store-length/store-klass, so concurrent GC
 135   // could see inconsistent values when scanning the old gen.

This comment is wrong for a single generation concurent collector.
Such a collector might still support TLAB allocation for performance.
It would just have arrange to not look in the TLABs when that would be
a problem.  Consider a SATB-based collector, where allocation is
black; do something like make TLAB portions from before start of mark
parsable, and don't scan objects in TLAB portions (possibly entire
TLABs) allocated from after start of marking.  I think Shenandoah
might be an example of this, and similarly for a collector I worked on
for a former employer.

------------------------------------------------------------------------------ 
src/share/vm/gc/shared/genCollectedHeap.cpp  
1019   assert(is_in_young((oop)result), "TLAB must be allocated in young gen");

I think this assertion is wrong, for the same reason I think the
comment for allocate_new_tlab is wrong.

------------------------------------------------------------------------------ 
src/share/vm/gc/shared/genCollectedHeap.cpp 
 492       assert(is_in_young((oop)result) ||
 493              ((oop)result)->klass_or_null() == NULL, 
 494              "mem_allocate must clear (what will be) the klass field for non-young allocations");

I think a more accurate assertion regimen would be to verify
non-humongous allocations were made from the young gen, and humongous
allocations have the needed null klass location.

------------------------------------------------------------------------------

I'm generally suspicious of the proposed comments and assertions.  I
think the actual contracts are not as simple :) as suggested by these
changes. 

For example, for G1, some of the relevant requirements around setup
and ordering on the allocation side appear to be to support concurrent
refinement.  If using the suggested G1 throughput mode (simpler
post-barrier, no concurrent refinement), some of those requirements
might be relaxed, though we might not conditionalize the code.

And for G1 generally, some of the ordering being done on the slow path
(such as ensuring the length of arrays and classes are set before
setting the klass field) is only needed when the object is humongous,
but again here the code isn't conditionalized.

So I think these kind of changes are premature.  I think what is first
needed is some discussion of what the contracts really are, e.g. reify
the design that is presently implicit in the code.  Ideally, this
should identify parts that are specific to certain GCs or GCs having
certain properties.  Once we have that, then we can start talking
about how to feed that information back into the code, in the form of
assertions, conditionalizations, &etc.

An additional benefit of this would be discovery of code bits that
might be conditionalized as part of the GC abstration layer that has
been discussed recently.  It might also help identify "shared" code
that is really CMS-specific, in support of CMS deprecation.