I'm looking at the log of the job you've run:
http://prt-web.us.oracle.com//archive/2013/07/2013-07-05-045326.ddaugher.8016838_exp/logs/linux_x64-product-c2-jdk_lang.log.FAILED.log
And it looks like both tests failed, not only the first one:
FAIL: Virtual memory usage increased by 1411072Kb (greater than 32768Kb)
FAIL: RedefineBigClassApp exited with status of 1
...
FAIL: Virtual memory usage increased by 1411072Kb (greater than 32768Kb)
FAIL: RetransformBigClassApp exited with status of 1
I have these two tests locally on 64-bit Linux and they both fail with similar
results in logs.
I may not tell for sure why the tests didn't fail on 32-bit Linux.
Whether it was because the /proc/self/stat approach didn't work or
because the leak is specific to 64-bit platform.
I have tested the RedefineBigClass test on JPRT with JDK6 (the code was
a bit different, but the approach was the same).
The test passed with JDK6u60 and failed (as expected) with JDK6u51 on
both 32 and 64-bit Linux machines.
I'm going to do more testings today.
Sincerely yours,
Ivan
On 05.07.2013 9:37, Daniel D. Daugherty wrote:
In a JPRT test job I just ran:
RedefineBigClass.sh passed on 9 platforms and failed on Linux 64-bit.
RetransformBigClass.sh passed on all 10 platforms.
Does your own testing only showing failure on the Linux 64-bit VM
and passed on the Linux 32-bit VM?
Dan
On 7/4/13 10:35 PM, Daniel D. Daugherty wrote:
Ivan,
The changes look fine, I can sponsor your commit, looks like your
OpenJDK user name is 'igerasim', but I need to know a little bit
more about your testing of these fixes. Did you do a test JPRT
job to run the JLI tests (or just the two tests themselves)?
Based on e-mail about this bug fix, I believe you've found a new
leak in JDK8/HSX-25 with test java/lang/instrument/RedefineBigClass.sh.
That's a good thing, but I think Alan and company would be a bit
grumpy with me if I pushed a test that failed as soon as someone
ran it. :-) Do you know if the revised RetransformBigClass.sh also
finds a new memory leak in JDK8/HSX-25?
The way to deal with that is to put the test on the Problem.list
as part of the same changeset. However, the T&L folks might not like
that either...
Dan
On 7/4/13 6:59 PM, Ivan Gerasimov wrote:
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
