Details
-
Bug
-
Resolution: Fixed
-
P4
-
8, 9, 10
-
b21
Description
Gunter Haug (gunter.haug@sap.com) provided the following bug report:
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.
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.