RFR(M): 8139867: Change how startsHumongous and continuesHumongous regions work in G1.

Tom Benson tom.benson at oracle.com
Wed Nov 4 14:35:02 UTC 2015


Hi,
I'm still working through the whole change, but have a comment/question 
about this:

On 11/4/2015 7:01 AM, David Lindholm wrote:
>>>    - can we add an assert in HeapRegion::block_is_obj() that the new 
>>> case
>>> can only occur with humongous objects, and that p must be in the same
>>> humongous object?
>> Ok, fixed.

The assertions imply it can handle an address *within* a humongous 
object, but I think it will only work for pointers to the object base:

  119   if (!this->is_in(p)) {
  120     HeapRegion* hr = g1h->heap_region_containing(p);
  121 #ifdef ASSERT
  122     assert(hr->is_humongous(), "This case can only happen for 
humongous regions");
  123     oop obj = oop(hr->humongous_start_region()->bottom());
  124     assert((HeapWord*)obj <= p, "p must be in humongous object");
  125     assert(p <= (HeapWord*)obj + obj->size(), "p must be in 
humongous object");
  126 #endif
  127     return hr->block_is_obj(p);
  128   }

I think if p != the base of the humongous_start_region(), we should 
probably just return false from block_is_obj. (Or assert, if you never 
expect an interior pointer).  Otherwise the recursive call might return 
true (if p < top()), and then HeapRegion::block_size will be unhappy 
when it tries to get the object size.   And the call to 
is_obj_dead(oop(p)...) will also be broken.
Am I missing something?   Tnx,
Tom



More information about the hotspot-gc-dev mailing list