RFR: 8229517: Support for optional asynchronous/buffered logging [v11]

[Thomas Stuefe]

On Fri, 7 May 2021 09:06:50 GMT, Xin Liu <xliu at openjdk.org> wrote:

> > The base class is unaware of the fact that you keep track of the list tail here. If used incorrectly (eg calling base class removal functions) you could damage this structure, because the tail node gets removed.
> 
> LinkedListDeque "private" inherits LinkedListImpl. They are not 'is-a' relationship. LinkedListDeque just borrows some code from LinkedListImpl in its implementation. Client can't sabotage "deque" integrity using LinkedList's API.

Okay, I see, I missed the private. 

> 
> Actually, my design target is a generic deque based on linkedlist. Linked list can give me pop_all() in O(1). you can think it of a smart `swap`.

I agree, that is fine, but you don't even need the move() operation. I mean, as Robbin proposed, you could just have two instances of this FIFO buffer somewhere and swap them around.

> 
> I acknowledge that problem you described. That's the cost to use ADT(Abstract Data Type). Even STL containers sometimes are cumbersome. I would like to hear other reviewers opinions about this. Shall I customize my own data structure or try to use ADT?
> 
> I have fixed most issues in linkedlist.hpp in prior commits. If @kimbarrett allows me to use move constructor, I can even remove `AsyncLogMessage::destroy`, which is just compromise to avoid copying.

I am very biased here because I often prefer handwritten code to these containers, since the complexity and runtime overhead often eats up the advantage of using generic code. That's just me, Kim may have a totally different opinion. Maybe its just this implementation. Eg I like very much the kernel list implementation, both generic and efficient.

In this case, using that ADT means:
- at least one unnecessary copy step when filling the list node
- one unnecessary dynamic allocation, since the node needs to be allocated. This is also one additional (well hidden) call to os::malloc, which is another barrier for ever making UL reentrant-safe wrt the hotspot allocation layer. 

But I wont fight you on this, especially since we already talked about improving this storage in some future RFE. So leave it in if you prefer.

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

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