stringStream in UL and nested ResourceMarks
thomas.stuefe at gmail.com
Wed Jun 7 09:15:59 UTC 2017
I saw this, but I also see LogStreamNoResourceMark being used as a default
for the (trace|debug|info|warning|error)_stream() methods of Log. In this
form it is used quite a lot.
Looking further, I see that one cannot just exchange
LogStreamNoResourceMark with LogStreamCHeap, because there are hidden usage
conventions I was not aware of:
LogStreamNoResourceMark is allocated with new() in create_log_stream().
LogStreamNoResourceMark is an outputStream, which is a ResourceObj. In its
current form ResourceObj cannot be deleted, so destructors for ResourceObj
child cannot be called.
So, we could not use malloc in the stringStream - or exchange stringStream
for bufferedStream - because we would need a non-empty destructor to free
the malloc'd memory, and that destructor cannot exist.
Looking further, I see that this imposes subtle usage restrictions for UL:
LogStreamNoResourceMark objects are used via "log.debug_stream()" or
similar. For example:
codecache_print(log.debug_stream(), /* detailed= */ false);
debug_stream() will allocate a LogStreamNoResourceMark object which lives
in the resourcearea. This is a bit surprising, because "debug_stream()"
feels like it returns a singleton or a member variable of log.
If one wants to use LogStreamCHeap instead, it must not be created with
new() - which would be a subtle memory leak because the destructor would
never be called - but instead on the stack as automatic variable:
I may understand this wrong, but if this is true, this is quite a difficult
API. I have two classes which look like siblings but LogStreamCHeap can
only be allocated on the local stack - otherwise I'll get a memory leak -
while LogStreamNoResourceMark gets created in the resource area, which
prevents its destructor from running and may fill the resource area up with
temporary stream objects if used in a certain way.
Have I understood this right so far? If yes, would it be possible to
Kind Regards, Thomas
On Wed, Jun 7, 2017 at 9:20 AM, Stefan Karlsson <stefan.karlsson at oracle.com>
> 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
>> 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
>> 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
>> is hit just in the right moment while printing. Which depends on the
>> 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
>> improved) bufferedStream? I also imagine this could make UL coding a bit
> Not answering your questions, but I want to point out that we already have
> a UL stream that uses C-Heap:
> // The backing buffer is allocated in CHeap memory.
> typedef LogStreamBase<bufferedStream> LogStreamCHeap;
>> Thank you,
>> Kind Regards, Thomas
More information about the hotspot-dev