RFR: 8292989: Avoid dynamic memory in AsyncLogWriter [v2]

[Xin Liu]

On Tue, 6 Sep 2022 16:23:02 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> src/hotspot/share/logging/logAsyncWriter.cpp line 75:
>> 
>>> 73:     p->set_message(dest);
>>> 74:     _pos += sz;
>>> 75:     return true;
>> 
>> That's pretty hacky... :-) And `set_message` only exists because you need to repair the message pointer here, right?
>> 
>> Proposal:
>> 
>> Make AsyncLogMessage self-contained and variable-sized by letting the message follow the header. Like this:
>> 
>> 
>> class AsyncLogMessage {
>>   LogFileStreamOutput* _output;
>>   const LogDecorations _decorations;
>>   const size_t _len; // optional, or could also be 32- maybe even 16 bit
>> public:
>>   size_t len_in_bytes() const { return align_up(sizeof(AsyncLogMessage) + _len); }
>>   const char* message() { return (const char*) (this + 1); }
>> };
>> 
>> 
>> Then give it a placement new() and place it inside the Buffer right away instead of creating it on the stack and then copying it to the buffer. Just an idea.
>
> Update: I looked at AsyncLogMessage and you are almost at what I propose anyway since the in-buffer version of AsyncLogMessage already follows my proposed layout. And your `AsyncLogMessage::size()` is similar to what I propose too. 
> 
> But the way you do it, you have two versions, one where the message follows the header, one where it is wherever. This means that `AsyncLogMessage::size()` is wrong for at least half the time.

I prefer trailing 0 because `AsyncLogMessage::message()` can be treated as a valid c-string and pass to LogFileStreamOutput. If so, `len` is optional indeed. The benefit of `len` is that we can iterate buffer faster. We will have O(1) time-complexity instead of O(len) in `Buffer::Iterator::next() `

I see your point. It's more elegant to use `implicit` message. The reason I need to handle `msg= nullptr` is that flushing Token uses it. I can replace it with "" instead.

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

PR: https://git.openjdk.org/jdk/pull/10092