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

[Daniel D. Daugherty]

Thanks for the clarification.

As a test fix, I also believe that JDK-8227041 can be pushed to jdk13.

Dan


On 7/17/19 10:31 AM, Langer, Christoph wrote:
> 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