RFR [8016838] java/lang/instrument/RedefineBigClass.sh needs modification

[Ivan Gerasimov]

Thank you, Daniel!

Please find an updated webrev at 
http://cr.openjdk.java.net/~igerasim/8016838/2/webrev/.
It now includes the RetransformBigClass test modified in the same way as 
RedefineBigClass.

If the changes look fine, may I ask you to sponsor the commit, as I'm 
not a committer?
Here's a link to hg export:
http://cr.openjdk.java.net/~igerasim/2commit/8016838-jdk8-ReBigClass-improved.patch

Thanks in advance,
Ivan

On 04.07.2013 21:45, Daniel D. Daugherty wrote:
> On 7/4/13 11:19 AM, Ivan Gerasimov wrote:
>> Daniel, thank you for review!
>>
>> Here's the updated with all all your suggestions adopted.
>> http://cr.openjdk.java.net/~igerasim/8016838/1/webrev/
>
> Looks good.
>
>
>>
>> I haven't yet considered applying the approach to RetransformBigClass.
>> Do you want me to include this into this same change set or should I 
>> make it separately?
>
> I would include it in the same changeset.
>
> Dan
>
>
>>
>> Sincerely yours,
>> Ivan
>>
>>
>> On 04.07.2013 19:34, Daniel D. Daugherty wrote:
>>> On 7/3/13 11:12 AM, Ivan Gerasimov wrote:
>>>> Hello everybody!
>>>>
>>>> We have a request to improve jtreg test.
>>>> The test had been written to verify fix for memory leak during 
>>>> class redefinition.
>>>> The problem is that it always is reported as PASSED even in the 
>>>> presence of the leak.
>>>>
>>>> The proposed change is platform specific.
>>>> It allows memory leak detection only on Linux.
>>>> This is because the memory leak was in native code, so there's no 
>>>> easy way to detect it from within Java code.
>>>>
>>>> Here's the webrev for JDK8 repository:
>>>> http://cr.openjdk.java.net/~igerasim/8016838/0/webrev/
>>>
>>> Very nicely done! The logic in RedefineBigClass.sh only catches a
>>> leak big enough to cause an Exception to be thrown.
>>>
>>> When I originally wrote this test and its companion:
>>>
>>>     test/java/lang/instrument/RetransformBigClass*
>>>
>>> I could not come up with a platform independent way to put a small
>>> upper limit on memory growth. It never dawned on me that putting in
>>> something on one platform (Linux) was better than nothing.
>>>
>>> Are you planning to add this same logic to RetransformBigClass*?
>>>
>>>
>>>
>>> test/java/lang/instrument/RedefineBigClass.sh
>>>     No comments.
>>>
>>> test/java/lang/instrument/RedefineBigClassApp.java
>>>     line 48: final long MEM_LEAK_THRESHOLD = 32 * 1024; // 32Mb
>>>         Would be better at the top of RedefineBigClassApp rather than
>>>         "buried" down here.
>>>
>>>     line 51: Long.valueOf(vmMemDelta)
>>>         There are several places where a long is passed to 
>>> Long.valueOf().
>>>         Is there some reason you're not passing them directly to 
>>> println()?
>>>
>>>     line 54: } else {
>>>         The "else" is redundant due to the "System.exit(1)" call 
>>> above it.
>>>         You can drop the "else {" and "}" and shift that last 
>>> println() to
>>>         the left.
>>>
>>>     line 71: return Long.parseLong(line.split(" ")[22]) / 1024;
>>>         How about at least a comment referring to the Linux proc(5)
>>>         man page... and the fact that the 23rd field is:
>>>
>>>             vsize %lu   Virtual memory size in bytes.
>>>
>>> Again, very nicely done!
>>>
>>> Dan
>>>
>>>
>>>>
>>>> The surprising thing is that modified test detected the leak with 
>>>> the latest JDK8!
>>>> Latest 6 and 7 are fine, so it might be regression in JDK8.
>>>>
>>>> I've filled a bug http://bugs.sun.com/view_bug.do?bug_id=8019845 
>>>> "Memory leak during class redefenition" (not yet available.)
>>>>
>>>> Sincerely yours,
>>>> Ivan Gerasimov
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>
>
>
>