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

[Thomas Stuefe]

On Fri, 26 Mar 2021 09:17:54 GMT, Volker Simonis <simonis at openjdk.org> wrote:

>> Xin Liu has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Change option AsyncLogBufferEntries to AsyncLogBufferSize.
>>   
>>   AsyncLogBufferSize is the memory budget in bytes for Asynchronous Logging.
>>   User can specify it in shorthand form. eg. -XX:AsyncLogBufferSize=10M.
>
> src/hotspot/share/logging/logAsyncFlusher.cpp line 81:
> 
>> 79: LogAsyncFlusher* LogAsyncFlusher::_instance = NULL;
>> 80: 
>> 81: void LogAsyncFlusher::initialize() {
> 
> I don't think you need this. Please merge the initialization into `LogAsyncFlusher::instance()` (see my comments on your changes in `init.cpp`).

In contrast to Volker I think an explicit initialization is fine and follows the established pattern for other subsystems. However, I would:
- assert for _instance == NULL and remove the condition. This should only be called once.
- no need for Atomics here since we do this during VM init.

And while writing this, I realize this would conflict with the gtest fixture where you switch this on or off several time in one process. I propose you switch those gtests - only those testing the LogAsyncFlusher, not those testing the Deque etc - as TEST_OTHER_VM. 

Which arguably we should do for all logging gtests, not only yours, if they fiddle with global VM settings. In other words, it should be possible to run the gtests with -Xlog:... options and the gtests should still pass, including all log tests. Today, this is not the case.

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

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