RFR: JDK-8264987: G1: Fill BOTs for Survivor-turned-to-Old regions in full gc

[Hamlin Li]

On Tue, 13 Apr 2021 12:32:51 GMT, Stefan Johansson <sjohanss at openjdk.org> wrote:

>> src/hotspot/share/gc/g1/g1FullGCPrepareTask.cpp line 190:
>> 
>>> 188:   }
>>> 189:   assert(next_addr == limit, "Should stop the scan at the limit.");
>>> 190: }
>> 
>> I think this could be implemented using `apply_to_marked_objects()`. Let the closure keep track of the threshold for the region and the `apply()` function could look something like this:
>> 
>> size_t apply(oop object) {
>>   size_t size = object->size();
>>   HeapWord* addr = cast_from_oop<HeapWord*>(object);
>>   HeapWord* next_addr = addr + size;
>>   if (next_addr > _threshold) {
>>     _threshold = hr->cross_threshold(addr, next_addr);
>>   }
>>   return size;
>> }
>
> I realize that we need to keep the previous address in the closure as well, for the case when we step over the threshold because of an unmarked object.

Hi Stefan, Thanks for the suggestion. 
At first thought, I think it's a good way to reuse the traversal ability of HeapRegion::apply_to_marked_objects.

But when I try to implement it actually, seems to me it might make the code a little complicated.

For the first part, in the new G1UpdateBotClosure, we need to indroduce an _pre_addr as you suggested. this part is implemented as below, it's not that complicated, please check the following code snippet:

 void G1FullGCPrepareTask::G1CalculatePointersClosure::update_bot(HeapRegion* hr) {
  G1UpdateBotClosure updateBot(hr);
  hr->apply_to_marked_objects(_bitmap, &updateBot);
 }

G1FullGCPrepareTask::G1UpdateBotClosure::G1UpdateBotClosure(HeapRegion* hr) :
    _hr(hr),
    _pre_addr(hr->bottom()),
    _threshold(hr->initialize_threshold()) { }

size_t G1FullGCPrepareTask::G1UpdateBotClosure::apply(oop object) {
  HeapWord* addr = cast_from_oop<HeapWord*>(object);
  size_t size = object->size();
  HeapWord* next_addr = addr + size;

  if(addr > _threshold) {
    _threshold = _hr->cross_threshold(_pre_addr, addr);
  }
  if (next_addr > _threshold) {
    _threshold = _hr->cross_threshold(addr, next_addr);
  }
  _pre_addr = next_addr;+  return size;
}



But, the above code does not consider the situation: one or several dead objects are at the end of the heap region, at this situation, we needs to update the bot outside of the G1UpdateBotClosure after hr->apply_to_marked_objects(...). I did not implement this part yet, but seems to me it makes the code a little complicated and not that readable, so would like to discuss with you first.

In summary, seems it does not make the code more readable, and will increase the complexity.
How do you think about it?

-------------

PR: https://git.openjdk.java.net/jdk/pull/3459