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

[Albert Mingkun Yang]

On Sat, 22 May 2021 09:10:27 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> This patch provides a buffer to store asynchrounous messages and flush them to
>> underlying files periodically.
>
> Xin Liu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update according to reviewers' feedback (Part-2).

src/hotspot/share/logging/logAsyncWriter.cpp line 35:

> 33: class AsyncLogLocker : public StackObj {
> 34:  private:
> 35:   debug_only(static intx _locking_thread_id;)

It's unclear to me what this variable is for.

src/hotspot/share/logging/logAsyncWriter.cpp line 129:

> 127:   AsyncLogBuffer logs;
> 128:   { // critical region
> 129:     AsyncLogLocker ml;

For consistency, I suggest `lock` as the name.

src/hotspot/share/logging/logAsyncWriter.cpp line 134:

> 132:     _buffer.pop_all(&logs);
> 133:     // append meta-messages of dropped counters
> 134:     _stats.iterate(&dropped_counters_iter);

Any particular reason why the iterator is *not* declared right above its usage? IOW, why not


_buffer.pop_all(&logs);
AsyncLogMapIterator dropped_counters_iter(logs);
_stats.iterate(&dropped_counters_iter);

src/hotspot/share/logging/logAsyncWriter.cpp line 152:

> 150:   _state = ThreadState::Running;
> 151: 
> 152:   while (_state == ThreadState::Running) {

`_state` would never change from now on, right? If so, `while (true)` should suffice, IMO.

src/hotspot/share/logging/logAsyncWriter.hpp line 137:

> 135: class AsyncLogWriter : public NonJavaThread {
> 136:   static AsyncLogWriter* _instance;
> 137:   // _sem is a semaphore whose value denotes how many messages have been enqueued.

"how many messages have been enqueued" seems to suggest it's a monotonically growing number as new message are enqueued. In fact, this number will decrease as `_buffer` is flushed. Maybe "_sem denotes the number of messages in _buffer` and moving the two together?

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

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