RFR: 8153658: Redundant memory copy in LogStreamNoResourceMark

Stefan Karlsson stefan.karlsson at oracle.com
Thu Apr 7 06:54:39 UTC 2016


Hi Kim,

On 2016-04-07 07:17, Kim Barrett wrote:
>> On Apr 6, 2016, at 1:30 PM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
>>
>> Hi all,
>>
>> Please review this patch to remove a redundant memory copy in the UL log stream classes.
>>
>> http://cr.openjdk.java.net/~stefank/8153658/webrev.01
>> https://bugs.openjdk.java.net/browse/JDK-8153658
>>
>> LogStreamNoResourceMark copies the resource allocated string buffer into a new resource allocated string buffer before copying the data to UL.
>>
>> Moreover, this also causes problem when implementing a log stream class using CHeap memory instead of Resource memory. Even though the first allocation is done from CHeap the second copy comes from Resource memory.
> ------------------------------------------------------------------------------
> src/share/vm/logging/logStream.inline.hpp
>    36     _current_line.write("\0", 1);
>
> I think this write isn't needed. stringStream purportedly ensures the
> internal buffer is NUL terminated, according to a comment in
> stringStream::write.  And the code seems to do that.

You're right. My intention is to be able to use this code together with 
a bufferedStream, which doesn't NULL terminate. See:
http://mail.openjdk.java.net/pipermail/hotspot-dev/2016-April/022477.html

So, this is the easiest way for me to be able to use the same code for 
both stringStream and bufferedStream.

>    Hm, except maybe
> at construction time?  That seems like a bug.  And I think bad things
> might happen if a stringStream is constructed with initial_size or
> fixed_buffer_size of zero, which would also be a bug.

stringStream::as_string() also NULL terminate, so I'm not sure this is a 
real bug. It is a weird inconsistency, though.

Thanks for reviewing,
StefanK

>
> ------------------------------------------------------------------------------
>



More information about the hotspot-dev mailing list