RFR: 8069367: assert(_nextMarkBitMap->isMarked((HeapWord*) obj)) failed
bengt.rutisson at oracle.com
Mon Apr 13 10:34:42 UTC 2015
On 11/04/15 07:51, Kim Barrett wrote:
> Another round.
> I've updated my local repo from the team repo, merging with the
> changes from 8076265: Simplify deal_with_reference.
> I've backed out the G1EagerReclaimHumongousPreSnapshotTypeArrays
> experimental configuration option, per Bengt's suggestion that it will
> likely be more trouble than it's worth.
> Because of the latter, the test to determine whether a humongous
> region should be a candidate for eager reclamation has been
> simplified, since at present there is no need to check the allocation
> time of the object. But I've left the commentary there about
> different handling of objects containing references, and noting the
> special handling of typeArray objects in this respect.
> Also had to move some new inline function definitions from
> g1CollectedHeap.hpp to g1CollectedHeap.inline.hpp, to account for
> header cleanups since my repo was last updated.
The latest webrev looks ok to me. I'm fine with pushing it as is.
However, I think it would have been easier to review this if it had been
split up to two changes. First a preparation fix that does the renaming
of _humongous_is_live to
_humongous_reclaim_candidates and the removal of
clear_humongous_is_live_table() and the other refactoring. Then one that
actually includes the fix, with process_grey_object() and the changes to
These two things are separable, right? Or am I missing something? As I
said I'm fine with pushing this as is, I just want to make sure that I
have understood correctly.
Some minor comments that you can feel free to ignore. I won't insist on
any of them:
The comment saying "Immediately process arrays of binary data" is a
little odd to me. I think I would prefer something like "Immediately
process arrays of primitive types".
1943 _hrm.reserved().start(), _hrm.reserved().end(),
1945 _hrm.reserved().start(), _hrm.reserved().end(),
When we line break long parameter lists we usually put one parameter per
line. The above would be:
See for example the call to
JavaThread::satb_mark_queue_set().initialize() a few lines below this code.
This comment in humongous_region_is_candidate() reads a bit strange to me.
3445 // However, we presently only nominate is_typeArray() objects.
3446 // A humongous object containing references induces remembered
3447 // set entries on other regions. In order to reclaim such an
3448 // object, those remembered sets would need to be cleaned up.
I think the main problem with reclaiming objects that contain references
is that they may be the only old objects that keep an object graph
alive. If new objects reference any object in that object graph we have
to be sure that they are kept alive. Otherwise we will have stale
pointers in the new objects.
No changes. This is probably just webrev acting up. For some reason it
includes files that have been modified and then reverted. But there
shouldn't be any changes to g1_globals.hpp, right?
Why is the test TestGreyReclaimedHumongousObjects excluded from the
> Incremental webrev:
> The incremental webrev might be a little misleading, since the base
> for it is not the version from the previous round of review, but
> rather that version plus the merge for 8076265.
> Hand testing
> Aurora Ad-hoc GC Nightly using G1
> local RefWorkload using G1
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the hotspot-gc-dev