<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<br>
<div class="moz-cite-prefix">On 3/29/14 12:03 AM, Andrey Zakharov
wrote:<br>
</div>
<blockquote cite="mid:53367041.5060203@oracle.com" type="cite">Hi,
here is updated webrev:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jwilhelm/8037925/webrev.04/">http://cr.openjdk.java.net/~jwilhelm/8037925/webrev.04/</a>
<br>
</blockquote>
<br>
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
<pre> 56 for (int i = 0; i < PagesNum; i++) {
57 ArrayList< byte[] > stuff = new ArrayList<>();
58 eat(stuff, 100, humonSize);
59 MemoryUsagePrinter.printMemoryUsage("eat #" + i);
60 garbage.add(stuff);
61 }
Seems like an out-of-memory (OOME) can be thrown here (or anywhere below
this block of code). Will an OOME be considered a pass, fail or
test-not-run?
Jon
</pre>
<br>
<blockquote cite="mid:53367041.5060203@oracle.com" type="cite">Thanks.
<br>
<br>
<br>
On 28.03.2014 18:03, Igor Ignatyev wrote:
<br>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">how should I know about it? you
didn't mention about it in your RFR
<br>
</blockquote>
Nop, I did. Please, check my first message.
<br>
</blockquote>
</blockquote>
Oh, I'm sorry, I need more sleep or coffee.
<br>
in future, you can use one patch for multiple fixes, it'll solve
such situation.
<br>
<br>
<blockquote type="cite"> 47 private static final
ArrayList< ArrayList< byte[] > > garbage = new
ArrayList< ArrayList< byte[] > >();
<br>
59 ArrayList< byte[] > stuff = new
ArrayList< byte[] >();
<br>
</blockquote>
redundant spaces, + use diamonds
<br>
<br>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite"> 26 * @key gc
<br>
</blockquote>
why do you need this?
<br>
</blockquote>
Hm, used as common practice.
<br>
</blockquote>
</blockquote>
</blockquote>
if you don't understand meaning of '@key gc', please don't use
it.
<br>
<br>
44-45,48-49:
<br>
According to section 9 in java code conventions[1]:
<br>
<blockquote type="cite">The names of variables declared class
constants and of ANSI constants should be all uppercase with
words separated by underscores ("_"). (ANSI constants should
be avoided, for ease of debugging.)
<br>
</blockquote>
<br>
[1]
<a class="moz-txt-link-freetext" href="http://www.oracle.com/technetwork/java/javase/documentation/codeconvtoc-136057.html">http://www.oracle.com/technetwork/java/javase/documentation/codeconvtoc-136057.html</a><br>
<br>
Igor
<br>
<br>
On 03/28/2014 05:35 PM, Andrey Zakharov wrote:
<br>
<blockquote type="cite">Hi,
<br>
here is updated webrev:
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jwilhelm/8037925/webrev.03/">http://cr.openjdk.java.net/~jwilhelm/8037925/webrev.03/</a>
<br>
<br>
Igor, please, see comments below.
<br>
Thanks.
<br>
<br>
On 28.03.2014 02:24, Igor Ignatyev wrote:
<br>
<blockquote type="cite">Andrey,
<br>
<br>
<blockquote type="cite">
<blockquote type="cite">Most probably I forgot TEST.groups
again...
<br>
</blockquote>
</blockquote>
yeap, you did.
<br>
</blockquote>
Ok, note that due I have two unpushed changes in TEST.groups I
have
<br>
change from previous webrev.
<br>
<blockquote type="cite">
<br>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">- I can't find
MemoryUsagePrinter class in testlibrary[1]
<br>
</blockquote>
It is in webrev:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jwilhelm/8037924/webrev/">http://cr.openjdk.java.net/~jwilhelm/8037924/webrev/</a>
<br>
</blockquote>
</blockquote>
how should I know about it? you didn't mention about it in
your RFR
<br>
</blockquote>
Nop, I did. Please, check my first message.
<br>
<blockquote type="cite">
<br>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">- please replace string
constant, which use in MemoryUsagePrinter, by
<br>
enum
<br>
</blockquote>
</blockquote>
</blockquote>
You don't need to use enum here and string constant
(Labels.*) in
<br>
TestDynShrinkHeap. I've mistakenly think that
MemoryUsagePrinter uses
<br>
String argument to change its own behavior. Sorry for
useless extra work.
<br>
</blockquote>
Yes, its just string labels and nothing more.
<br>
<br>
<blockquote type="cite">
<br>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">- lines 79,82: why did you use
<br>
sun.management.ManagementFactoryHelper.getDiagnosticMXBean()
instead
<br>
of
<br>
'ManagementFactory.getPlatformMXBean(HotSpotDiagnosticMXBean.class)'
?
<br>
</blockquote>
Are we talking about this?
<br>
throw new RuntimeException(
<br>
String.format("committed free heap
size is not
<br>
less
<br>
than committed full heap size, heap hasn't been
shrunk?%n"
<br>
+ "%s = %s%n%s = %s",
<br>
MinFreeRatioFlagName,
<br>
ManagementFactory.getPlatformMXBean(HotSpotDiagnosticMXBean.class).
<br>
getVMOption(MinFreeRatioFlagName).getValue(),
<br>
MaxFreeRatioFlagName,
<br>
ManagementFactory.getPlatformMXBean(HotSpotDiagnosticMXBean.class).
<br>
getVMOption(MaxFreeRatioFlagName).getValue()
<br>
)
<br>
</blockquote>
</blockquote>
yes, we're.
<br>
</blockquote>
Ok, changed.
<br>
<blockquote type="cite">
<br>
please, use generics in 54,65 lines:
<br>
<blockquote type="cite">54 private static final List
garbage = new ArrayList();
<br>
65 List stuff = new ArrayList();
<br>
</blockquote>
and then, you won't have to use cast in line 95:
<br>
<blockquote type="cite"> 95 ArrayList stuff =
((ArrayList) garbage.get(garbage.size()
<br>
- 1));
<br>
</blockquote>
<br>
</blockquote>
Done
<br>
<blockquote type="cite">
<blockquote type="cite"> 26 * @key gc
<br>
</blockquote>
why do you need this?
<br>
</blockquote>
Hm, used as common practice.
<br>
<blockquote type="cite">
<br>
<blockquote type="cite"> 64 for (int i = 0; i <
5; i++) {
<br>
</blockquote>
magic number '5'
<br>
</blockquote>
Good point, changed
<br>
<blockquote type="cite">
<br>
<blockquote type="cite"> 61 int humonSize =
Math.round(1f * PageSize);
<br>
</blockquote>
I'd prefer '1.0f', but it's only my fad, it's not necessary
to change.
<br>
<br>
Thanks
<br>
Igor
<br>
<br>
On 03/27/2014 04:37 PM, Andrey Zakharov wrote:
<br>
<blockquote type="cite">Hi, here is updated webrev:
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jwilhelm/8037925/webrev.02/">http://cr.openjdk.java.net/~jwilhelm/8037925/webrev.02/</a>
<br>
Most probably I forgot TEST.groups again...
<br>
<br>
<br>
On 26.03.2014 23:12, Igor Ignatyev wrote:
<br>
<blockquote type="cite">Hi Andrey,
<br>
<br>
- I can't find MemoryUsagePrinter class in
testlibrary[1]
<br>
</blockquote>
It is in webrev:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jwilhelm/8037924/webrev/">http://cr.openjdk.java.net/~jwilhelm/8037924/webrev/</a>
<br>
<blockquote type="cite">- please replace string constant,
which use in MemoryUsagePrinter, by
<br>
enum
<br>
- you forgot to remove commented line 53 Hi
<br>
</blockquote>
Done
<br>
<blockquote type="cite">- lines 79,82: why did you use
<br>
sun.management.ManagementFactoryHelper.getDiagnosticMXBean()
instead
<br>
of
<br>
'ManagementFactory.getPlatformMXBean(HotSpotDiagnosticMXBean.class)'
?
<br>
</blockquote>
Are we talking about this?
<br>
throw new RuntimeException(
<br>
String.format("committed free heap
size is not less
<br>
than committed full heap size, heap hasn't been shrunk?%n"
<br>
+ "%s = %s%n%s = %s",
<br>
MinFreeRatioFlagName,
<br>
ManagementFactory.getPlatformMXBean(HotSpotDiagnosticMXBean.class).
<br>
getVMOption(MinFreeRatioFlagName).getValue(),
<br>
MaxFreeRatioFlagName,
<br>
ManagementFactory.getPlatformMXBean(HotSpotDiagnosticMXBean.class).
<br>
getVMOption(MaxFreeRatioFlagName).getValue()
<br>
)
<br>
);
<br>
<blockquote type="cite">
<br>
- just a nit: you again use extra spaces in some
brackets:
<br>
55,68,61,74,..
<br>
</blockquote>
Done
<br>
Thanks.
<br>
<blockquote type="cite">
<br>
[1]
<br>
<a class="moz-txt-link-freetext" href="http://hg.openjdk.java.net/jdk9/hs-gc/hotspot/file/tip/test/testlibrary/com/oracle/java/testlibrary">http://hg.openjdk.java.net/jdk9/hs-gc/hotspot/file/tip/test/testlibrary/com/oracle/java/testlibrary</a>
<br>
<br>
<br>
<br>
Thanks,
<br>
Igor
<br>
<br>
On 03/26/2014 11:02 PM, Andrey Zakharov wrote:
<br>
<blockquote type="cite">Test to check, that if we
allocate h1, h2 and h3 humongous objects and
<br>
free h1 and h2, after GC G1 shrinks the heap.
<br>
webrev:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jwilhelm/8037925/webrev/">http://cr.openjdk.java.net/~jwilhelm/8037925/webrev/</a>
<br>
bug: <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8037925">https://bugs.openjdk.java.net/browse/JDK-8037925</a>
<br>
<br>
This test used testlibrary MemoryUsagePrinter from
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jwilhelm/8037924/webrev/">http://cr.openjdk.java.net/~jwilhelm/8037924/webrev/</a>
<br>
<br>
Thanks.
<br>
<br>
</blockquote>
</blockquote>
<br>
</blockquote>
</blockquote>
<br>
</blockquote>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>