RFR: 8276229: Stop allowing implicit updates in G1BlockOffsetTable

[Stefan Johansson]

On Wed, 10 Nov 2021 11:58:59 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Please review this change that removes some code that is no longer needed.
>> 
>> **Summary**
>> In [JDK-8272083](https://bugs.openjdk.java.net/browse/JDK-8276229) G1 was changed to create a precise and complete BOT during evacuation (in the GC pause). This removes the need to update the BOT when looking up block starts. This previously happened in concurrent refinement and while scanning the heap in the pause, but with the precise BOT we should no longer require any such updates.
>> 
>> This change removes the slow path function that did the updates `forward_to_block_containing_addr_slow(...)`. It also renames some functions to no longer include a `_const` post-fix because there are only "const" versions of the functions remaining.
>> 
>> **Testing**
>> 
>> - [x] Mach5 1-3
>> - [x] Local stress-testing
>
> src/hotspot/share/gc/g1/g1BlockOffsetTable.inline.hpp line 147:
> 
>> 145:     assert(_bot->index_for(n) == _bot->index_for(addr),
>> 146:            "BOT not precise. Index for n: " SIZE_FORMAT " must be equal to the index for addr: " SIZE_FORMAT,
>> 147:            _bot->index_for(n), _bot->index_for(addr));
> 
> I think this method is unnecessary, and always returns `q`. In `block_start()`, the only caller afaics, we already made sure that `addr` is < `top()`, i.e. we always ever ask for addresses where the BOT is complete.
> For a complete BOT, 
> 
>   HeapWord* q = block_at_or_preceding(addr);
>   HeapWord* n = q + block_size(q);
> 
> `q` is necessarily the closest start of the block, and `n` always > `addr`.
> It would be different if `block_start` allowed queries after `top`, then we would need to skip at most one block as mentioned in the comment, but then I'd just write:
> ``` if (addr >= top()) { return _top; }```
> 
> Unless I'm missing something crucial here?

You're missing the case where `addr` is not covered by the first object ("block") in the card. So when asking for `block_at_or_preciding(addr)` we get an object that is stretching into the card same card as `addr` map to, but it is not certain that it is the object containing `addr`.

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

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