RFR: 8213352: Separate BufferNode allocation from PtrQueueSet

Kim Barrett kim.barrett at oracle.com
Wed Nov 7 23:55:07 UTC 2018


> On Nov 7, 2018, at 6:22 PM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Hi,
> 
> On Sun, 2018-11-04 at 23:51 -0500, Kim Barrett wrote:
>> Please review this change to PtrQueue buffer allocation, moving the
>> implementation of free-list based allocation from PtrQueueSet to a
>> separate BufferNode::Allocator object.  The old code supported
>> sharing
>> of free-lists by having one PtrQueueSet delegate to another; now
>> free-list sharing is provided by using the same allocator object.
>> 
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8213352
>> 
>> Webrev:
>> http://cr.openjdk.java.net/~kbarrett/8213352/open.00/
>> 
>> Testing:
>> mach5 tier1-5
>> Added gtest for new allocator class.
>> 
> 
>  - copyright for the gtest should be 2018 only

Copy-paste from an existing test. Thanks for spotting it.

>  - out of curiosity, not an issue in the code, but why the need for
> Atomic::load() in BufferNode::Allocator::free_count? It's okay, but I
> do not see what Atomic::load provides you in this case compared to a
> normal load. Is this the new preferred style to load from a volatile
> variable of primitive type?

Maybe I shouldn’t do that, or maybe I should be proselytizing it.
I’m finding the use of Atomic::(load|store) helpful by being explicit about
relaxed loads/stores, rather than just “knowing" that’s what’s happening
because the variable is declared volatile.  I’ve just recently started doing
this sometimes in code I’m working on; this might be the first RFR with
any of that.  There are currently only 31 other uses of those functions.

> Don't need to see the new webrev for the copyright date change.

Thanks.




More information about the hotspot-gc-dev mailing list