RFR: 8276229: Stop allowing implicit updates in G1BlockOffsetTable

[Stefan Johansson]

On Wed, 10 Nov 2021 13:20:28 GMT, Stefan Johansson <sjohanss at openjdk.org> wrote:

>> 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`.

My initial thought was also that this should not be needed, but the code proved me wrong. It might be possible to refactor the code even further to avoid this case.

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

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