RFR: JDK-8262068: Improve G1 Full GC by skipping compaction for regions with high survival ratio [v14]

[Thomas Schatzl]

On Thu, 8 Apr 2021 10:02:49 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> src/hotspot/share/gc/g1/g1FullCollector.cpp line 229:
>> 
>>> 227: 
>>> 228: void G1FullCollector::update_attribute_table(HeapRegion* hr) {
>>> 229:   if (hr->is_free()) {
>> 
>> Another item that has been noted in a recent discussion with @albertnetymk is that with this change "Free" regions are also marked as `normal` in the table. It would be better to keep them as "Invalid".
>> 
>> I.e. something like (incorporating @albertnetymk other suggestion):
>> 
>>     if (hr->is_free()) {
>>       return;
>>     } else if (hr->is_closed_archive(...) {
>>       [...]
>>     } else if (hr->is_pinned() || force_pinned) {
>>       [...]
>>     } else {
>>       [...]
>>     }
>> 
>> There is no real difference as "Free" regions should never be referenced anywhere and the code should assert elsewhere. It's still nice to also have "Free" regions as `Invalid` in that table though.
>
> not exactly. please check previous discussion at https://github.com/openjdk/jdk/pull/2760#discussion_r602144952.

Some more investigation and discussion about the BOT handling showed that we need to update the BOT for these Survivor-turned-to-Old regions after all.

The reason is that contrary to what I thought, while BOT can handle queries for object start addresses above the "last known valid entry" mentioned in PR#3356, it only does so slowly, and while updating the BOT itself, not updating that "last known valid entry".
So every time it queries for an object start in such regions, it starts walking from the bottom of that region.

See the call chain `G1BlockOffsetTablePart::block_start` -> `forward_to_block_containing_addr` -> `forward_to_block_containing_addr_slow` where the call to `alloc_block_work` in `g1BlockOffsetTable.cpp:236` only updates local boundary and index (not `_next_offset_threshold` and `_next_offset_index`).

This is a problem for young gcs as this behavior will make them slower than expected. Since it is impossible to make the updates to both `_next_offset_threshold` and `_next_offset_index` atomic, and another issue anyway) I would prefer to penalize full gc (or at least keep old behavior) for that.

Could you add code that walks such full young regions and does the `cross_threshold` thing? It certainly does not need to actually compact these regions.

Thanks,
  Thomas

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

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