RFR: 8017163: G1: Refactor remembered sets [v4]

[Thomas Schatzl]

On Mon, 31 May 2021 10:02:18 GMT, Stefan Johansson <sjohanss at openjdk.org> wrote:

>> Thomas Schatzl has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   More cleanup after sjohanss comments
>
> src/hotspot/share/gc/g1/g1ServiceThread.cpp line 29:
> 
>> 27: #include "gc/g1/heapRegion.inline.hpp"
>> 28: #include "gc/g1/heapRegionRemSet.inline.hpp"
>> 29: #include "gc/shared/suspendibleThreadSet.hpp"
> 
> I think all changes to g1ServiceThread.* could be reverted now when you added the state to the new task instead. Adding `is_enqueued()` to not check `next() == NULL` is a good change, but it should be made on its own.

I agree. Note that I think that these changes fix real issues (no virtual destructor with virtual methods) too.

> src/hotspot/share/gc/g1/heapRegionRemSet.hpp line 126:
> 
>> 124:       // This correction is necessary because the above includes the second
>> 125:       // part.
>> 126:       + (sizeof(HeapRegionRemSet) - sizeof(G1CardSet))
> 
> Pre-existing, but what do you think about moving/including the comment for this part of the calculation in the function comment.

I think the comment should stay where it is - it is confusing otherwise, and establishing the context for this seems hard. I can remove the comment if you want though.

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

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