(ping) RFR(xs): 8146905 - cleanup ostream, staticBufferStream

Thomas Stüfe thomas.stuefe at gmail.com
Mon Jan 25 09:30:50 UTC 2016


Hi all,

I would still like a second review and a sponsor on this one.

This is both a cleanup (Bug https://bugs.openjdk.java.net/browse/JDK-8146905)
and a fix for (https://bugs.openjdk.java.net/browse/JDK-8146905): it only
affects the stream used for hs_err files, so its effect on the rest of the
VM is limited.

Current webrev:
http://cr.openjdk.java.net/~stuefe/webrevs/8146905-get-rid-of-staticBufferStream/webrev.01/webrev/

Thanks and Regards, Thomas



On Wed, Jan 20, 2016 at 10:29 AM, Thomas Stüfe <thomas.stuefe at gmail.com>
wrote:

> Thank you Volker!
>
> Still need a second reviewer, and once hs-rt is open again I'll need a
> sponsor too.
>
> Kind Regards, Thomas
>
> On Wed, Jan 20, 2016 at 9:32 AM, Volker Simonis <volker.simonis at gmail.com>
> wrote:
>
>> Hi Thomas,
>>
>> thanks for doing this nice cleanup.
>>
>> The change looks good. Can you please just adapt the comment at line
>> 987 and remove the reference to staticBufferStream from there as well.
>>
>> Regards,
>> Volker
>>
>>
>> On Wed, Jan 20, 2016 at 8:54 AM, Thomas Stüfe <thomas.stuefe at gmail.com>
>> wrote:
>> > May I please have a review for this? Thanks!
>> >
>> > On Wed, Jan 13, 2016 at 3:00 PM, Thomas Stüfe <thomas.stuefe at gmail.com>
>> > wrote:
>> >
>> >> Dear all,
>> >>
>> >> please take a look at this small cleanup of ostream.
>> >>
>> >> bug report: https://bugs.openjdk.java.net/browse/JDK-8146905
>> >> webrev:
>> >>
>> http://cr.openjdk.java.net/~stuefe/webrevs/8146905-get-rid-of-staticBufferStream/webrev.00/webrev/
>> >>
>> >> Basically, it gets rid of the staticBufferStream class by moving its
>> one
>> >> feature up to the parent outputStream class:
>> >>
>> >> When printing the error log in vmError.cpp, we want to use as little
>> stack
>> >> space as possible. outputStream class uses on-stack buffers to assemble
>> >> snprintf functions. So, staticBufferStream was introduced as a child of
>> >> outputStream which overwrites the print functions and makes them use a
>> >> caller provided buffer. It then delegates the real writing to a
>> connected
>> >> stream object.
>> >>
>> >> The problem with that approach is that this is not a good fit for a
>> child
>> >> class.Not all operations possible with outputStream are cleanly
>> delegated
>> >> to the connected stream class, so a staticBufferStream behaves
>> sometimes
>> >> differently from all other streams (see e.g. JDK-8145410, which will be
>> >> automatically fixed with this change too).
>> >>
>> >> Another problem is that this delegation model leads to some code
>> >> duplication, because all print() methods of outputStream are
>> practically
>> >> duplicated in staticBufferStream.
>> >>
>> >> Another cosmetic note is that all other child classes of outputStream
>> >> (bufferedStream, stringStream, fileStream...) are specializations in
>> term
>> >> of log destination, not internal behaviour.
>> >>
>> >> Note that I implemented this in a very simple way without using
>> templates,
>> >> because I wanted to keep the changes as small as possible.
>> >>
>> >> Kind Regards, Thomas
>> >>
>>
>
>


More information about the hotspot-runtime-dev mailing list