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

[Xin Liu]

On Wed, 19 May 2021 12:49:30 GMT, Volker Simonis <simonis at openjdk.org> wrote:

>> Xin Liu has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Implement the new discard policy: drop the incoming message.
>>   
>>   This patch also fix a bug. meta messages append to the temp logs instead
>>   of directly calling output->write_blocking(). This guranatees logsites are non-blocking.
>
> src/hotspot/share/logging/logAsyncWriter.cpp line 129:
> 
>> 127:   // Along with logs destruction, all procceeded messages are deleted.
>> 128:   //
>> 129:   // the atomic operation 'move' is done in O(1). All I/O jobs are done without lock.
> 
> Start with uppercase letter. Replace 'move' by 'pop_all()' because at this point it in´s not obvious that 'pop_all()' eventually calls 'LinkedList::move()'.
> What do you mean with "atomic" here? Maybe better remove?

` _buffer.pop_all(&logs)` is placed in 'critical region' on purpose. 
It's because logsites may be enqueuing concurrently. that's why I need to use a lock to protect _buffer. 
I see 'atomic' is inappropriate in the context. I will remove it.

> src/hotspot/share/logging/logAsyncWriter.cpp line 175:
> 
>> 173:     if (self->_state == ThreadState::Initialized) {
>> 174:       Atomic::release_store_fence(&AsyncLogWriter::_instance, self);
>> 175:       // all readers of _instance after fence see NULL,
> 
> Please start sentences with uppercase letter.
> 
> I don't understand this? Wouldn't all readers of '_instance' see 'self' *after* it was stored there with 'Atomic::release_store_fence()`? Or do you wanted to say *before* the fence here?

My fault.  I mean "All readers of _instance after the fence see non-NULL".

> src/hotspot/share/logging/logAsyncWriter.hpp line 89:
> 
>> 87: 
>> 88:   LinkedListNode<E>* head() const {
>> 89:     return static_cast<const LinkedList<E>* >(this)->head();
> 
> Why not simply `return this->_head`?

yes, it works!

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

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