RFR: 8274987 G1: reuse the newly allocated G1SegmentedArrayBuffer even if current thread failed the race [v3]

[Thomas Schatzl]

On Wed, 20 Oct 2021 09:48:35 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> This is a follow-up of JDK-8273626.
>> 
>> Current (original) implementation just drops the newly allocated G1SegmentedArrayBuffer if current thread fails race, but seems reuse (e.g adding to freelist) will bring some benefit.
>
> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix cransh on Mac; remove unnecessary code

Changes requested by tschatzl (Reviewer).

src/hotspot/share/gc/g1/g1SegmentedArray.inline.hpp line 70:

> 68: template<MEMFLAGS flag>
> 69: void G1SegmentedArrayBufferList<flag>::add(G1SegmentedArrayBuffer<flag>& elem) {
> 70:   _list.prepend(elem);

Not sure why the `prepend` is used here, but probably because of the previous implementation which has been dead code.
I do not see an advantage pushing this element onto the bottom of the stack. I.e. I'd just `push()` it.

Note that this change (in general) also breaks the assumptions stated in the documentation of `G1SegmentedArrayBufferList` - that's not a problem as far as I can see (push/pop use the correct synchronization), but the text needs to be adapted.

src/hotspot/share/gc/g1/g1SegmentedArray.inline.hpp line 87:

> 85:   G1SegmentedArrayBuffer<flag>* result = _list.pop();
> 86:   if (result != nullptr) {
> 87:     Atomic::sub(&_num_buffers, (size_t)1,memory_order_relaxed);

This is to use the same method in this and the next line? Anyway, there should be a space before the `memory_order_relaxed`. Since `add()` also uses `inc()` this does not seem consistent. Or is there another reason? I would just keep it as is.

src/hotspot/share/gc/g1/g1SegmentedArray.inline.hpp line 143:

> 141:   if (old != prev) {
> 142:     next->set_next(nullptr);
> 143:     // reuse the newly allocated or poped buffer by adding it into free buffer list.

s/poped/popped

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

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