RFR: 8064909: FragmentMetaspace.java got OutOfMemoryError

[Jon Masamitsu]

Michail,

Thanks for making the changes.  Looks good.

Reviewed.

Jon

On 12/08/2014 03:12 AM, Michail Chernov wrote:
> Hi,
>
> Comment was changed according to Jon's message:
> http://cr.openjdk.java.net/~eistepan/~mchernov/8064909/webrev.02/
>
> Thanks,
> Michail
>
> On 01.12.2014 22:52, Jon Masamitsu wrote:
>>
>> On 11/27/2014 05:28 AM, Michail Chernov wrote:
>>> Hi,
>>>
>>> CC'ed hotspot-runtime-dev.
>>>
>>> Here is not test failure - test works as expected. OOME is occurred 
>>> in compiler instance.
>>>
>>> private JavaCompiler javac;
>>> ...
>>>         javac = ToolProvider.getSystemJavaCompiler();
>>> ...
>>>         int exitcode = javac.run(null, null, null, 
>>> file.getCanonicalPath());
>>>         if (exitcode != 0) {
>>>             throw new RuntimeException("javac failure when 
>>> compiling: " +
>>>         file.getCanonicalPath());
>>>
>>> Here is 2 ways - rewrite getGeneratedClass 
>>> (runtime/testlibrary/GeneratedClassLoader.java) to allow them to 
>>> throw not only RuntimeException,
>>
>> Seems like this would be more precise with regard to recognizing the
>> cause of the failure.  Are there too many places which would have to
>> change to catch the OOME.
>>
>>> or to catch RuntimeException and check exception message comparing 
>>> with "javac failure when compiling:". Both ways seem to me are not 
>>> as clear as expected for this simple test. More - javac does not 
>>> throw anything - it just returns exitcode (non-zero) and writes its 
>>> messages to System.err.
>>>
>>> Also I can add comment to code like "OOME with message 
>>> "java.lang.OutOfMemoryError: Java heap space" doesn't mean that 
>>> something wrong with metaspace - need just to increase -Xmx".
>>
>> That would be enough for me if you don't think
>> throwing the OOME from  GeneratedClassLoader()
>> adds much value.
>>
>> Jon
>>
>>>
>>> Thanks,
>>> Michail
>>>
>>> On 27.11.2014 2:00, Jon Masamitsu wrote:
>>>> Dima,
>>>>
>>>> If this test fails with an OOME in the future, I would like it to be
>>>> obvious that the failure is not that an OOME occurred.   I cannot
>>>> tell that from looking at the test.    Can the test be changed so
>>>> I don't have to spend time figuring out that the OOME is not
>>>> a failure mode  of the test?
>>>>
>>>> Jon
>>>>
>>>>
>>>> On 11/26/2014 12:51 PM, Dmitry Fazunenko wrote:
>>>>> Hi Jon,
>>>>>
>>>>> The original version of test worked for 80 seconds trying to 
>>>>> perform as many iterations as possible. The number of iterations 
>>>>> performed depended on how fast is the machine. With each next 
>>>>> iteration the size of generated and loaded classes is growing, so 
>>>>> on fast enough machines 80 seconds is enough to run out of heap 
>>>>> while generating a class.
>>>>>
>>>>> The fix not only sets the heap, but limits iterations. 300m heap 
>>>>> is enough for 200 iterations.
>>>>>
>>>>> Your approach, with catching OOME(heap) and passing will also 
>>>>> work, but it will reduce the test readability (and potentially 
>>>>> could bring more problems).
>>>>>
>>>>> An alternative approach would be to limit metaspace and heap 
>>>>> accordingly and load classes until we don't run out metaspace... 
>>>>> But this might take awhile.
>>>>>
>>>>> So, I hope that Michael's fix is good.
>>>>>
>>>>> Thanks for looking and expressing comments.
>>>>> Dima
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On 26.11.2014 22:04, Jon Masamitsu wrote:
>>>>>> Michail,
>>>>>>
>>>>>> Your change makes this test pass but it seems like at
>>>>>> some future date 300m might not be big enough
>>>>>> (for whatever reason).  Could the test be make to
>>>>>> caught an OOME, print out a message saying that
>>>>>> an OOME doesn't mean the test failed  but that
>>>>>> the test needs a larger heap?  Then pass an
>>>>>> exception up (maybe some type of Runtime
>>>>>> exception - sorry if that is vague but I don't
>>>>>> what type of exception would make sense).  That
>>>>>> would mean we wouldn't have to spend time
>>>>>> diagnosing what the OOME means again.
>>>>>>
>>>>>> Jon
>>>>>>
>>>>>> On 11/26/2014 5:36 AM, Michail Chernov wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Please review this simple fix for nightly test failure:
>>>>>>>
>>>>>>> Webrev:
>>>>>>> http://cr.openjdk.java.net/~eistepan/~mchernov/8064909/webrev.00/
>>>>>>> Bug:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8064909
>>>>>>>
>>>>>>> Problem: test fails because of OOME (not enough heap size).
>>>>>>> Solution: heap size were increased.
>>>>>>>
>>>>>>> Testing:
>>>>>>> jtreg
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Michail
>>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>
>>
>>
>