stringStream in UL and nested ResourceMarks

[Stefan Karlsson]

Hi Thomas,

On 2017-06-06 11:40, Thomas Stüfe wrote:
> Hi all,
> 
> In our VM we recently hit something similar to
> https://bugs.openjdk.java.net/browse/JDK-8167995 or
> https://bugs.openjdk.java.net/browse/JDK-8149557:
> 
> A stringStream* was handed down to nested print functions which create
> their own ResourceMarks and, while being down the stack under the scope of
> that new ResourceMark, the stringStream needed to enlarge its internal
> buffer. This is the situation the assert inside stringStream::write()
> attempts to catch (assert(Thread::current()->current_resource_mark() ==
> rm); in our case this was a release build, so we just crashed.
> 
> The solution for both JDK-816795 and JDK-8149557 seemed to be to just
> remove the offending ResourceMarks, or shuffle them around, but generally
> this is not an optimal solution, or?
> 
> We actually question whether using resource area memory is a good idea for
> outputStream chuild objects at all:
> 
> outputStream instances typically travel down the stack a lot by getting
> handed sub-print-functions, so they run danger of crossing resource mark
> boundaries like above. The sub functions are usually oblivious to the type
> of outputStream* handed down, and as well they should be. And if the
> allocate resource area memory themselves, it makes sense to guard them with
> ResourceMark in case they are called in a loop.
> 
> The assert inside stringStream::write() is not a real help either, because
> whether or not it hits depends on pure luck - whether the realloc code path
> is hit just in the right moment while printing. Which depends on the buffer
> size and the print history, which is variable, especially with logging.
> 
> The only advantage to using bufferedStream (C-Heap) is a small performance
> improvement when allocating. The question is whether this is really worth
> the risk of using resource area memory in this fashion. Especially in the
> context of UL where we are about to do expensive IO operations (writing to
> log file) or may lock (os::flockfile).
> 
> Also, the difference between bufferedStream and stringStream might be
> reduced by improving bufferedStream (e.g. by using a member char array for
> small allocations and delay using malloc() for larger arrays.)
> 
> What you think? Should we get rid of stringStream and only use an (possibly
> improved) bufferedStream? I also imagine this could make UL coding a bit
> simpler.

Not answering your questions, but I want to point out that we already 
have a UL stream that uses C-Heap:

logging/logStream.hpp:

// The backing buffer is allocated in CHeap memory.
typedef LogStreamBase<bufferedStream> LogStreamCHeap;

StefanK

> 
> Thank you,
> 
> Kind Regards, Thomas
>