RFR(XS): 8175900: Assertion too strict in G1CollectedHeap::new_mutator_alloc_region

Volker Simonis volker.simonis at gmail.com
Mon Feb 27 17:10:35 UTC 2017


can I please have a review and sponsor for the following tiny change:


Gunter Haug (gunter.haug at sap.com) provided the following bug report and fix:

The assertion (!force || g1_policy()->can_expand_young_list()) in
G1CollectedHeap::new_mutator_alloc_region appears to be too strict. In
fact, new_mutator_alloc_region with force=true is called from
attempt_allocation_slow only under the condition of
g1_policy()->can_expand_young_list() indirectly, via this hierarchy:

G1CollectedHeap::new_mutator_alloc_region(unsigned long, bool)
  MutatorAllocRegion::allocate_new_region(unsigned long, bool)
    G1AllocRegion::new_alloc_region_and_allocate(unsigned long, bool)
      G1AllocRegion::attempt_allocation_force(unsigned long, bool)
        G1CollectedHeap::attempt_allocation_slow(unsigned long,
unsigned char, unsigned int*, int*)

However, this happens in a mutator thread while the
G1YoungRemSetSamplingThread is running concurrently and may call
revise_young_list_target_length_if_necessary(), which in turn calls
update_max_gc_locker_expansion() where _young_list_max_length may be
decreased and therefor g1_policy()->can_expand_young_list() is not
true anymore.

This behavior is rare, we only observed it a few times in our nightly
tests over several years. However, by suspending the mutator thread in
attempt_allocation_slow, for a few seconds before calling
attempt_allocation_force, we were able to reproduce the behavior.

We'd like to propose to remove the assertion.

Also, _young_list_max_length and the other counters in G1DefaultPolicy
which are accessed concurrently should be declared volatile. But that
should be handled in an separate issue.

Thank you and best regards,

More information about the hotspot-gc-dev mailing list