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