<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<br>
<div class="moz-cite-prefix">On 02/02/2016 02:09 PM, Kirill
Zhaldbybin wrote:<br>
</div>
<blockquote cite="mid:56B12928.1070703@oracle.com" type="cite">Jon,
<br>
<br>
Thank you for reviewing the fix.
<br>
<br>
On 02/02/2016 10:53 PM, Jon Masamitsu wrote:
<br>
<blockquote type="cite">
<br>
Kirill,
<br>
<br>
Would it make sense to add another full GC after line 177?
<br>
</blockquote>
I think you are right - it could make the test more stable.
<br>
I also added additional output to separate this full GC from ones
that are called after deallocations.
<br>
<br>
Here are a new webrev:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8132721/webrev.06/">http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8132721/webrev.06/</a>
<br>
</blockquote>
<br>
Sorry I didn't catch this sooner but at lines 150 and 176<br>
<br>
"no GC should <font color="#ff0000">happened</font>"<br>
<br>
should be<br>
<br>
"no GC should <font color="#ff0000">happen</font>"<br>
<br>
Otherwise, looks good. I don't need another<br>
webrev.<br>
<br>
Jon<br>
<br>
<br>
<blockquote cite="mid:56B12928.1070703@oracle.com" type="cite">
<br>
Regards, Kirill
<br>
<blockquote type="cite">
<br>
<br>
<blockquote type="cite"> 175
<br>
176 System.out.println("Finished allocations - no GC
should happened before this line");
<br>
177
<br>
178
<br>
179 allocations.stream().forEach(allocation -> {
<br>
180 long usedMemoryBefore =
memoryCounter.getUsedMemory();
<br>
181 allocation.forgetAllocation();
<br>
182
<br>
183 WHITE_BOX.fullGC();
<br>
</blockquote>
<br>
It would remove some uncertainty in the start of the heap for
the
<br>
first iteration of the loop at 179.
<br>
<br>
Jon
<br>
<br>
On 2/2/2016 7:37 AM, Kirill Zhaldybin wrote:
<br>
<blockquote type="cite">Dmitry,
<br>
<br>
Thank you for reviewing this test.
<br>
<br>
Here are a new webrev:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8132721/webrev.05/">http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8132721/webrev.05/</a>
<br>
<br>
Could you please read comments inline?
<br>
<br>
Regards, Kirill
<br>
<br>
On 02.02.2016 15:25, Dmitry Fazunenko wrote:
<br>
<blockquote type="cite">Hi Kirill,
<br>
<br>
In general the test looks good.
<br>
<br>
A few minor comments:
<br>
<br>
1) copyrights 2015 -> 2016
<br>
</blockquote>
Fixed
<br>
<blockquote type="cite">2) Would you consider to move
<br>
gcBeans.stream().mapToLong(GarbageCollectorMXBean::getCollectionCount).sum();
<br>
<br>
into a separate method. You use this construct twice.
<br>
</blockquote>
Well, I think twice is still ok and takes less lines that
separate function.
<br>
<blockquote type="cite">
<br>
3) You print debug info:
<br>
<br>
System.out.println("Counter returned that
allocation used "
<br>
+ (usedMemoryAfter - usedMemoryBefore) + "; "
<br>
+ "expected allocation size at least "
+
<br>
expectedAllocationSize * ALLOCATION_SIZE_TOLERANCE_FACTOR);
<br>
<br>
for the sake of simplification debug in case of failure,
it's better
<br>
to print separately:
<br>
- allocationSize, usedMemoryBefore, usedMemoryAfter.
<br>
</blockquote>
Fixed.
<br>
<br>
<blockquote type="cite">
<br>
4)
<br>
<br>
if (gcCountNow == gcCountBefore) {
<br>
...
<br>
} else {
<br>
System.out.println("GC happened during
allocation so
<br>
the check is skipped");
<br>
}
<br>
<br>
In case of an unexpected GC happens on iteration #3, all the
following
<br>
iterations will be skipped.
<br>
But you can skip only iteration #3. For that in the 'else'
branch you
<br>
need to add
<br>
gcCountBefore = gcCountNow
<br>
</blockquote>
Fixed.
<br>
<br>
<blockquote type="cite">
<br>
Thanks,
<br>
Dima
<br>
<br>
<br>
<br>
On 01.02.2016 22:04, Kirill Zhaldybin wrote:
<br>
<blockquote type="cite">Dear all,
<br>
<br>
Could you please take a look at this test for JDK-8132721?
<br>
<br>
The test allocates/deallocates humongous objects and
checks that
<br>
memory counters (from Runtime and MemoryMXBean classes)
work right.
<br>
<br>
For example it's expected that after allocating HO of size
<br>
(G1Region/2 + 1) bytes free memory should decrease on full
G1Region
<br>
size.
<br>
<br>
CR:
<br>
<a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8132721">https://bugs.openjdk.java.net/browse/JDK-8132721</a>
<br>
<br>
WebRev:
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8132721/webrev.04/">http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8132721/webrev.04/</a>
<br>
<br>
<br>
Thank you.
<br>
Regards, Kirill
<br>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>