RFR(XL): 8181917: Refactor UL LogStreams to avoid using resource area

Erik Helin erik.helin at oracle.com
Tue Jun 27 12:53:45 UTC 2017


On 06/21/2017 06:16 PM, Thomas Stüfe wrote:
> New Webrev:
>  http://cr.openjdk.java.net/~stuefe/webrevs/8181917-refactor-ul-logstream/all.webrev.02/webrev/

I have spent most of my time in log.hpp, logStream.{hpp,cpp}, so I will 
start with those comments/questions and then continue reviewing the 
callsites.

log.hpp:
- two extra newlines near end of file

logStream.hpp:
- one extra newline near end of file
- should you add declarations of delete as well to prevent someone from
   calling delete on LogStream pointer? Thinking about this, it seems a
   bit odd to pass a function an outputStream* (a pointer to a
   ResourceObj) that doesn't obey the ResourceObj contract. An
   outputStream* coming from &ls, where ls is an LogStream instance on
   the stack, is a pointer to a ResourceObj that in practice is not a
   ResourceObj.

   Thinking about this some more, I think this is safe. No function that
   is passed an outputStream* can assume that it can call delete on that
   pointer.

- is 128 bytes as default too much for _smallbuf? Should we start with
   64?
- the keyword `public` is repeated unnecessarily LogStream class (a
   superfluous `public` before the write method)

logStream.cpp
- one extra newline at line 74
- the pointer returned from os::malloc is not checked. What should we
   do if a NULL pointer is returned? Print whatever is buffered and then
   do vm_out_of_memory?
- is growing by doubling too aggressive? should the LineBuffer instead
   grow by chunking (e.g. always add 64 more bytes)?
- instead of growing the LineBuffer by reallocating and then copying
   over the old bytes, should we use a chunked linked list instead (in
   order to avoid copying the same data multiple times)? The only
   "requirements" on the LineBuffer is fast append and fast iteration
   (doesn't have to support fast index lookup).
- LogStream::write is no longer in inline.hpp, is this a potential
   performance problem? I think not, ,the old LogStream::write
   definition was most likely in .inline.hpp because of template usage

Great work thus far Thomas, the patch is becoming really solid! If you 
want to discuss over IM, then you can (as always) find me in #openjdk on 
irc.oftc.net.

Thanks,
Erik

> Kind Regards, Thomas


More information about the hotspot-dev mailing list