8227041 (was 8225200): runtime/memory/RunUnitTestsConcurrently.java has a memory leak - push to jdk13?

[Langer, Christoph]

Hi Dan,

sorry, I just replied to the original thread which has the 8225200 bug id in its subject line for whatever reason. But the bug to backport is JDK-8227041 of course.

However, 8225200 has no associated patch anyway as Thomas thinks that it is resolved by 8227041, too.

Best regards
Christoph

> -----Original Message-----
> From: Daniel D. Daugherty <daniel.daugherty at oracle.com>
> Sent: Mittwoch, 17. Juli 2019 16:23
> To: Langer, Christoph <christoph.langer at sap.com>; Thomas Stüfe
> <thomas.stuefe at gmail.com>; Coleen Phillmore
> <coleen.phillimore at oracle.com>; Stefan Karlsson
> <stefan.karlsson at oracle.com>
> Cc: HotSpot Open Source Developers <hotspot-dev at openjdk.java.net>
> Subject: Re: 8225200: runtime/memory/RunUnitTestsConcurrently.java has a
> memory leak - push to jdk13?
> 
> The subject has the following bug ID: 8225200
> 
>      JDK-8225200 assert(vs.actual_committed_size() >= commit_size) failed
> 
> and a synopsis from a different bug:
> 
>      JDK-8227041 runtime/memory/RunUnitTestsConcurrently.java has a
> memory leak
> 
> Please clarify which you would like to backport...
> 
> Dan
> 
> 
> 
> On 7/17/19 10:10 AM, Langer, Christoph wrote:
> > Hi,
> >
> > as we're running into this issue in our nightly test environment, I would be
> very interested to bring this thing to jdk13. As per RDP rules
> (https://openjdk.java.net/jeps/3) , we are currently transitioning from RDP1
> to RDP2. But in both phases, it is allowed to push test fixes. So, would you say
> this is a test fix and can be pushed while still adhering to the rules? I'd say
> yes, but I'd like to get some confirmation (or rejection if I'm wrong...)
> >
> > That would be the change to push:
> http://hg.openjdk.java.net/jdk/jdk/rev/8a153a932d0f
> >
> > Thanks
> > Christoph
> >
> >
> >> -----Original Message-----
> >> From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net> On Behalf
> Of
> >> Thomas Stüfe
> >> Sent: Montag, 1. Juli 2019 21:19
> >> To: Coleen Phillmore <coleen.phillimore at oracle.com>
> >> Cc: HotSpot Open Source Developers <hotspot-dev at openjdk.java.net>
> >> Subject: Re: RFR(xs): 8225200:
> >> runtime/memory/RunUnitTestsConcurrently.java has a memory leak
> >>
> >> Thanks Coleen!
> >>
> >> On Mon, Jul 1, 2019, 21:14 <coleen.phillimore at oracle.com> wrote:
> >>
> >>> +1
> >>> Thank you for taking care of this!
> >>> Coleen
> >>>
> >>> On 7/1/19 3:07 PM, Thomas Stüfe wrote:
> >>>> Thanks Stefan!
> >>>>
> >>>> On Mon, Jul 1, 2019, 21:06 Stefan Karlsson
> <stefan.karlsson at oracle.com>
> >>>> wrote:
> >>>>
> >>>>> On 2019-07-01 20:56, Thomas Stüfe wrote:
> >>>>>> Hi all,
> >>>>>>
> >>>>>> may I please have reviews and opinions about the following patch:
> >>>>>>
> >>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8227041
> >>>>>> cr:
> >>>>>>
> >>> http://cr.openjdk.java.net/~stuefe/webrevs/8227041-
> >> rununittestsconcurrently-has-a-mem-
> leak/webrev.00/webrev/index.html
> >>>>>> There is a memory leak in test_virtual_space_list_large_chunk(),
> called
> >>>>> as
> >>>>>> part of the whitebox tests WB_RunMemoryUnitTests(). In this test
> >>>>> metaspace
> >>>>>> allocation is tested by rapidly allocating and subsequently leaking a
> >>>>>> metachunk of ~512K. This is done by a number of threads in a tight
> >> loop
> >>>>> for
> >>>>>> 15 seconds, which usually makes for 10-20GB rss. Test is usually OOM
> >>>>> killed.
> >>>>>> This test seems to be often excluded, which makes sense, since this
> >>> leak
> >>>>>> makes its memory usage difficult to predict.
> >>>>>>
> >>>>>> It is also earmarked by Oracle for gtest-ification, see 8213269.
> >>>>>>
> >>>>>> This leak is not easy to fix, among other things because it is not
> >>> clear
> >>>>>> what it is it wants to test. Meanwhile, time moved on and we have
> >> quite
> >>>>>> nice gtests to test metaspace allocation (see e.g.
> >>>>>> test_metaspace_allocation.cpp) and I rather would run those gtests
> >>>>>> concurrently. Which could be a future RFE.
> >>>>>>
> >>>>>> So I just removed this metaspace related test from
> >>>>> WB_RunMemoryUnitTests()
> >>>>>> altogether, since to me it does nothing useful. Once you remove the
> >>>>> leaking
> >>>>>> allocation, not much is left.
> >>>>>>
> >>>>>> Without this part RunUnitTestsConcurrently test runs smoothly
> >> through
> >>> its
> >>>>>> other parts, and in that form it is still useful.
> >>>>>>
> >>>>>> What do you think?
> >>>>> I think this makes sense and it looks good to me.
> >>>>>
> >>>>> Thanks,
> >>>>> StefanK
> >>>>>
> >>>>>> Cheers, Thomas
> >>>