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

Xin Liu xliu at openjdk.java.net
Fri Apr 23 22:11:33 UTC 2021


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

>> Xin Liu has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 18 additional commits since the last revision:
>> 
>>  - Refactor LogAsyncFlusher::abort()
>>    
>>    This change makes sure LogAsyncFlusher::abort() is lock-less.
>>    Therefore, it's not subject to mutex rank issue. Newly added
>>    gtest(mutex_lock_access_leaf)  may deliberately trigger SIGSEGV
>>    while holding access rank mutex, so abort() must be lockless.
>>  - Merge branch 'master' into JDK-8229517
>>  - Fix a race condition bug on LogAsyncFlusher termination.
>>    
>>    I saw intermitent crashes of java with the following arguments.
>>    -Xlog:'all=trace:file=hotspot-x.log:level,tags:filecount=0,async=true' --version
>>    
>>    The root cause is that there is a race condition between main thread _exit and
>>    LogAsyncFlusher::run. This patch added a synchronization using
>>    Terminator_lock in LogAsyncFlusher::terminate. This guranatees that no
>>    log entry will emit when main thread is exiting.
>>  - Resolve rank conflict between tty_lock and LogAsyncFlusher's _lock.
>>    
>>    LogAsyncFlusher::_lock ranks Mutex::tty on purpose, which is same as tty_lock.
>>    Ideally, they are orthogonal. In reality, it's possible that a thread emits logs
>>    to a log file while (mistakenly) holding tty_lock. ttyUnlocker is placed in enqueue
>>    member functions to resolve conflicts betwen them.
>>    
>>    This patch fixed the jtreg test: runtime/logging/RedefineClasses.java and the
>>    full brunt logging -Xlog:'all:file=hotspot.log::async=true'
>>  - Remove LogAsyncInterval completely
>>    
>>    It was used as a timeout parameter of the monitor. Now the monitor is waked up
>>    when the occupancy of asynclog buffer is more 3/4.
>>  - Support Jcmd pid VM.log disable
>>    
>>    This patch also supports to add a new output dynamically. If
>>    output_option specifies async=true, the new output will use
>>    asynchronous writing.
>>    
>>    Currently jcmd VM.log prohibts users from changing established
>>    output_options in runtime. users can disable them all and then
>>    recreate them with the new output_options.
>>  - Fix build issue with `--disable-precompiled-headers`
>>  - Inject the number of dropped messages since last dumpping.
>>    
>>    Each LogOutput has an independent counter. The out-of-band message
>>    "[number] messages dropped..." will be dumped into its corresponding
>>    LogOutput.
>>  - Revert "fix runtime/logging/RedefineClasses.java crashed with -XX:+AsyncLogging"
>>    
>>    This reverts commit 81b2a0cb2a6cf57b1cd0baacdf8c0419f14819b4.
>>    
>>    This problem is sidetracked by JDK-8265102.
>>  - fix runtime/logging/RedefineClasses.java crashed with -XX:+AsyncLogging
>>    
>>    nmethod::print(outputStream* st) should not obtain tty_lock by assuming
>>    st is defaultStream. It could be logStream as well.
>>    
>>    Currently, AyncLogFlusher::_lock has the same rank of tty_lock.
>>    https://issues.amazon.com/issues/JVM-563
>>  - ... and 8 more: https://git.openjdk.java.net/jdk/compare/1938a54a...4cf4a57f
>
> src/hotspot/share/logging/logAsyncFlusher.hpp line 111:
> 
>> 109:     o._message = NULL; // transfer the ownership of _message to this
>> 110:   }
>> 111: 
> 
> Maybe add an explicit copy assignment operator with a `ShouldNotReachHere` to make sure `AsyncLogMessages` are not assigned unintentionally.

I do use copy constructor here.  The container  ` typedef LinkedListDeque<AsyncLogMessage, mtLogging> AsyncLogBuffer;` stores objects instead of raw pointers on purpose.  I create objects and push it to the container in equeue().

It's always easier to manage memory with copying byval. There's no smart_pointer in hotspot, I had to explicitly  manage memory if I would store raw pointers.  That makes code error-prone and messy.

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

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


More information about the hotspot-dev mailing list