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

[Thomas Stuefe]

On Fri, 7 May 2021 06:13:19 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:
> 
>   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.

Hi Xin,

continuing the review. I concentrated on startup and termination/abort.

---

One general thing, I think it would be good to limit this to:
- we do initialize only once, if needed, during VM startup.
- we terminate only once, during VM shutdown.
In other words, exclude the possibility to switch async state at runtime, eg via jcmd. I think this would make matters simpler.

---

About synchronization: I don't understand the reasoning behind having a periodic flush. Would an implementation which just flushes if output is available not be less complex and actually better? Why make the flusher thread wake up twice a second if there is nothing to do? OTOH why wait 500ms if there is actually output to be printed?

IMHO this would make sense both for dense printing and sparse printing. When printing a lot, we'd keep the flusher busy without artificial waits. When not much happens, flusher does not need to be scheduled.

Furthermore, IIUC this wait introduces an artificial 500ms delay on VM shutdown, since you have this handshake with the flusher thread which does an unconditional wait. I don't think we should wait here, especially not for a thread which puts in an artificial delay.

Thanks, Thomas

p.s. code is well written und good to understand.

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

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