RFR(M): 8235998: [c2] Memory leaks during tracing after '8224193: stringStream should not use Resouce Area'.

[Lindenmaier, Goetz]

Hi David, Vladimir,

stringStream is a ResourceObj, thus it lives on an arena. 
This is uncritical, as it does not resize.
8224193 only changed the allocation of the internal char*,
which always caused problems with resizing under 
ResourceMarks that were not placed for the string but to 
free other memory.
Thus stringStream must not be deallocated, and 
also there was no mem leak before that change.
But we need to call the destructor to free the char*.

Best regards,
  Goetz.

> -----Original Message-----
> From: hotspot-runtime-dev <hotspot-runtime-dev-bounces at openjdk.java.net>
> On Behalf Of David Holmes
> Sent: Mittwoch, 18. Dezember 2019 04:33
> To: Vladimir Kozlov <vladimir.kozlov at oracle.com>; hotspot-compiler-
> dev at openjdk.java.net
> Cc: hotspot-runtime-dev at openjdk.java.net runtime <hotspot-runtime-
> dev at openjdk.java.net>
> Subject: Re: RFR(M): 8235998: [c2] Memory leaks during tracing after
> '8224193: stringStream should not use Resouce Area'.
> 
> On 18/12/2019 12:40 pm, Vladimir Kozlov wrote:
> > CCing to Runtime group.
> >
> > For me the use of `_print_inlining_stream->~stringStream()` is not obvious.
> > I would definitively miss to do that if I use stringStreams in some new
> > code.
> 
> But that is not a problem added by this changeset, the problem is that
> we're not deallocating these stringStreams even though we should be.  If
> you use a stringStream in new code you have to manage its lifecycle.
> 
> That said why is this:
> 
>   if (_print_inlining_stream != NULL)
> _print_inlining_stream->~stringStream();
> 
> not just:
> 
> delete _print_inlining_stream;
> 
> ?
> 
> Ditto for src/hotspot/share/opto/loopPredicate.cpp why are we explicitly
> calling the destructor rather than calling delete?
> 
> Cheers,
> David
> 
> > May be someone can suggest some C++ trick to do that automatically.
> > Thanks,
> > Vladimir
> >
> > On 12/16/19 6:34 AM, Lindenmaier, Goetz wrote:
> >> Hi,
> >>
> >> I'm resending this with fixed bugId ...
> >> Sorry!
> >>
> >> Best regards,
> >>    Goetz
> >>
> >>> Hi,
> >>>
> >>> PrintInlining and TraceLoopPredicate allocate stringStreams with new and
> >>> relied on the fact that all memory used is on the ResourceArea cleaned
> >>> after the compilation.
> >>>
> >>> Since 8224193 the char* of the stringStream is malloced and thus
> >>> must be freed. No doing so manifests a memory leak.
> >>> This is only relevant if the corresponding tracing is active.
> >>>
> >>> To fix TraceLoopPredicate I added the destructor call
> >>> Fixing PrintInlining is a bit more complicated, as it uses several
> >>> stringStreams. A row of them is in a GrowableArray which must
> >>> be walked to free all of them.
> >>> As the GrowableArray is on an arena no destructor is called for it.
> >>>
> >>> I also changed some as_string() calls to base() calls which reduced
> >>> memory need of the traces, and added a comment explaining the
> >>> constructor of GrowableArray that calls the copyconstructor for its
> >>> elements.
> >>>
> >>> Please review:
> >>> http://cr.openjdk.java.net/~goetz/wr19/8235998-
> c2_tracing_mem_leak/01/
> >>>
> >>> Best regards,
> >>>    Goetz.
> >>