[jdk17] RFR: 8267952: async logging supports to dynamically change tags and decorators

Xin Liu xliu at openjdk.java.net
Thu Jun 24 04:33:28 UTC 2021


On Wed, 23 Jun 2021 20:18:32 GMT, Xin Liu <xliu at openjdk.org> wrote:

> This patch fixed intermittent crashes of gtest/AsyncLogGtest.java. The root cause 
> is that previous flush() can't guarantee flush all pending messages in AsyncLog
> buffer. This patch implements flush() using a synchronizaton token and waits for
> completion. This approach isn't MT-safe but it can serialize all flush() calls in
> a thread. 
> 
> Two concurrent tests are appended to cover the hazard cases.
> This patch also comments LogConfiguration::configure_output() MT-safety.

hi, David, 
> I have one remaining concern, which is just something we need to keep an eye on, and that is the potential for delaying VM termination if the async log writer gets blocked/delayed.

We discussed this before. Yes, if "write_blocking()" does block on I/O part, flush() is blocked.  We have this issue before, and synchronous logging has this issue as well.    I think Async logging is better than sync logging on this because at least JVM got stuck at termination instead of any safepoint if it did happen.  

It isn't perfect. Ideally, we can use flush_sem.wait(timeout) if Semaphore::wait supports timeout for all platforms. 

> I'm curious now about the fact the buffer size seems only a logical size constraint not an actual out-of-space constraint.

We can have an assertion like this. It's because flush() must be thread-safe. The reason I don't enforce it using mutex or yet another semaphore is that current use cases have already been protected. If all flush() are serialized, it means that there's one or none token at any time. therefore, the following assertion is right. Shall I put it back? 

`
assert(_buffer.size() < _buffer_max_size+1, "_buffer is over-sized.");
`

>  Warning: asynclog is OFF.
It is from gtest fixture AsyncLogTest, which is expected. We run those tests in two modes. 
Normally, gtest run this test in synchronous mode. It doesn't test much. 

https://github.com/openjdk/jdk/blob/master/test/hotspot/gtest/logging/test_asynclog.cpp#L37

Thank you for you patience. TBH, I coundn't wrap around my head if you did tell me that handshake problem. 
I will post a new revision to include your feedbacks.

thanks,
--lx

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

PR: https://git.openjdk.java.net/jdk17/pull/130


More information about the hotspot-runtime-dev mailing list