RFR: 8145934: Make ttyLocker equivalent for Unified Logging framework

[Thomas Stüfe]

Hi Marcus,

On Thu, Feb 11, 2016 at 4:46 PM, Marcus Larsson <marcus.larsson at oracle.com>
wrote:

> Hi Thomas,
>
> On 02/11/2016 09:27 AM, Thomas Stüfe wrote:
>
> Hi Marcus,
>
> thank you for implementing this!
>
>
> Thanks for looking at it!
>
>
> I am happy that you implement this with a temporary buffer rather than
> implementing a lock.
>
> I did not yet look closely at all changes, this is quite a large change.
> But I have some questions about the memory you use:
>
> You use either resource area or NMT-tracked C heap. In both cases this is
> quite "high level memory" in the sense that it uses a number of components.
> These components then cannot use logging (or at least the LogMessage)
> without introducing circular references. Worse, this may often work and
> only crash if the LogMessage::grow() function grows the buffer, so it is
> difficult to test.
>
> Would it not be better to use plain low-level raw malloc in this case?
>
> Or, alternativly, factor memory allocation out into an own interface to be
> used as template parameter, so that most people get a reasonable default:
>
> LogMessage myMessage;
> ..
>
> but low level components like NMT, for example, could do:
>
> LogMessage<MyRawMallocator> myMessageUsingRawMalloc;
>
> ?
>
>
> Good point. This is actually already an issue with the current
> implementation for regular log messages. If they are longer than what the
> stack buffer in Log<>::vwrite() can hold, a buffer will be allocated on the
> heap instead (NMT tracked). I'm considering to approach this as a follow-up
> issue though, given that it's a pre-existing issue and that it won't be a
> problem until logging for NMT or similar is added.
>
>
This sounds reasonable.

>
> A second concern with using ResourceArea: One has to be careful with
> handing down LogMessage objects to subroutines, to fill the LogMessage
> object, and further down the callstack happens another ResourceMark
> destroying the LogMessage buffer. Could that be a problem, and if yes,
> could we have an assert for this case?
>
>
> Yes, there is a problem here if the LogMessage buffer grows under a
> different ResourceMark. I think it's possible to add an assert here, but I
> wonder if we don't actually want something more robust than that. Perhaps
> skipping the resource allocations altogether is a better idea.
>
>
Yes, maybe this is not necessary. In our VM we have something very similar
to LogMessage for our tracing system, but we just use raw malloc. So, we
traded in performance, but the code got simpler and more robust.

Kind Regards, Thomas



> Thanks,
> Marcus
>
>
> Kind Regards, Thomas
>
>
>
>
>
>
> On Wed, Feb 10, 2016 at 2:49 PM, Marcus Larsson <marcus.larsson at oracle.com
> > wrote:
>
>> Hi,
>>
>> Please review the following patch adding support for non-interleavable
>> multi-line log messages in UL.
>>
>> Summary:
>> This patch adds a LogMessage class that represents a multiline log
>> message, buffering lines that belong to the same message. The class has a
>> similar interface to the Log class, with printf-like methods for each log
>> level. These methods will append the log message with additional lines.
>> Once all filled in, the log message should be sent to the the appropriate
>> log(s) using Log<>::write(). All lines in the LogMessage are written in a
>> way that prevents interleaving by other messages. Lines are printed in the
>> same order they were added to the message (regardless of level). Apart from
>> the level, decorators will be identical for lines in the same LogMessage,
>> and all lines will be decorated.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~mlarsson/8145934/webrev.00/
>>
>> Issue:
>> https://bugs.openjdk.java.net/browse/JDK-8145934
>>
>> Testing:
>> Included tests through JPRT
>>
>> Thanks,
>> Marcus
>>
>
>
>