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

[Langer, Christoph]

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
> > >>
> >
> >