Request for sponsor - JDK-8031538

[Staffan Friberg]

Found this old thread lying around.

How about the following patch?

Checking the calculate_young_list_desired_min_length which simply just 
adds the base_min_length I think changing the base_min_length to contain 
more than survivors might increase the nursery size unexpectedly. This 
will keep the calculation the same, while still making sure we can't set 
the young size below what the it currently is.

@@ -538,13 +538,13 @@
    // This is how many young regions we already have (currently: the 
survivors).
    uint base_min_length = recorded_survivor_regions();
    // This is the absolute minimum young length, which ensures that we
-  // can allocate one eden region in the worst-case.
-  uint absolute_min_length = base_min_length + 1;
+  // will at least have one eden region available in the young list.
+  // If we shrink the young list target it won't shrink below the 
current size.
+  uint eden_length = _g1->young_list()->length() - 
recorded_survivor_regions();
+  uint absolute_min_length = base_min_length + (eden_length == 0) ? 1 : 
eden_length;
    uint desired_min_length =
calculate_young_list_desired_min_length(base_min_length);
-  if (desired_min_length < absolute_min_length) {
-    desired_min_length = absolute_min_length;
-  }
+  desired_min_length = MAX2(desired_min_length, absolute_min_length);

    // Calculate the absolute and desired max bounds.

//Staffan

> Hi again,
>
> On Thu, 2014-01-23 at 10:28 +0100, Thomas Schatzl wrote:
>> Hi,
>>
>> On Wed, 2014-01-22 at 16:56 -0800, Staffan Friberg wrote:
> [..]
>>> [1] - https://bugs.openjdk.java.net/browse/JDK-8031538
>> I think there is a small problem after the change: when revising the
>> young target list length concurrently, the code in lines 550/551 more or
>> less make sure that the young gen will only ever increase because
>> absolute_min_length is always young_list_length() + 1.
>>
>> So this could mean that if this length is updated really frequently, we
>> may exhaust the entire heap. E.g. recalculation of this length increases
>> the amount of eden -> allocation can continue -> before OOM there is
>> another recalculation -> goto start.
>>
>>   542   // This is how many young regions we already have (survivors +
>>         // eventual eden)
>>   543   // The young list contain all survivors regions and any eden regions
>>
>> Maybe add an explanation that during a GC pause the young list contains
>> only survivor regions at this point.
>>
>>   544   uint base_min_length = _g1->young_list()->length();
>>   545   // This is the absolute minimum young length, which ensures that we
>>   546   // can allocate one eden region in the worst-case.
>>   547   uint absolute_min_length = base_min_length + 1;
>>
>> I think the unconditional +1 is wrong. It should probably only add this single region if there are not yet eden regions added. I think you could get this information (whether the young list already contains an eden region) from YoungList, by subtracting length() with survivor_length(). If the result is > 0, there are eden regions in the list already. It's possibly best to add a method that returns that boolean value.
> Another possible fix is
>
>    uint absolute_min_length = _g1->recorded_survivor_regions() + 1; (or
> _g1->young_list()->survivor_regions().
>
> Did not think about what is better yet though (or whether they have
> issues of their own). Both solutions would not require additional code.
>
> Thomas
>
>