<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<tt>Thanks Dima for taking a look!<br>
<br>
I fixed all your comments and will include them when pushing.<br>
<br>
Stefan<br>
<br>
</tt>
<div class="moz-cite-prefix">On 2016-05-30 13:59, Dmitry Fazunenko
wrote:<br>
</div>
<blockquote
cite="mid:3aec3ce2-35ee-47e4-8bad-cfff307d99de@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
Hi Stefan,<br>
<br>
> Full: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esjohanss/8157153/hotspot.01/">http://cr.openjdk.java.net/~sjohanss/8157153/hotspot.01/</a>
<br>
<br>
The fix looks good to me. Some minor comments:<br>
<br>
<pre><span class="changed"><span class="changed">310 storage.setArrayAt((to * K + k), new Object[N]);
</span>--></span><span class="changed"></span><span class="changed"><span class="changed">
310 storage.setArrayAt(to * K + k, new Object[N]);</span></span>
<span class="changed"><span class="changed"></span></span></pre>
<br>
<pre><span class="new">355 public Object[][] storage;
-->
</span><span class="new">355 public final Object[][] storage;
</span><span class="new">363 Object[] getForIndex(int y) {</span>
<span class="new">364 return storage[y % usedCount];</span>
<span class="new">365 }
- no need this method.
</span>
<span class="new"><span class="new">368 return usedCount == storage.length;</span>
-->
</span><span class="new"><span class="new">368 return usedCount >= storage.length;</span>
</span>
I don't need a separate review for that tiny changes.
Thanks,
Dima
</pre>
<br>
<div class="moz-cite-prefix">On 27.05.2016 15:48, Stefan Johansson
wrote:<br>
</div>
<blockquote cite="mid:57484215.6060606@oracle.com" type="cite"> <br>
<br>
On 2016-05-27 13:33, Mikael Gerdin wrote: <br>
<blockquote type="cite">Hi Stefan, <br>
<br>
On 2016-05-26 17:20, Stefan Johansson wrote: <br>
<blockquote type="cite">Hi, <br>
<br>
Please review this testfix for: <br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8157153">https://bugs.openjdk.java.net/browse/JDK-8157153</a>
<br>
<br>
Webrev: <br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esjohanss/8157153/hotspot.00/">http://cr.openjdk.java.net/~sjohanss/8157153/hotspot.00/</a>
<br>
</blockquote>
<br>
Would it be possible to encapsulate the storage[] so that you
don't have to put the "% storageUsedCount" all over the place?
<br>
This seems a bit fragile to me. <br>
</blockquote>
Thanks for taking a look Mikael. I agree, that my initial fix
might cause problems and I like how your idea turned out: <br>
Full: <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esjohanss/8157153/hotspot.01/">http://cr.openjdk.java.net/~sjohanss/8157153/hotspot.01/</a>
<br>
Inc: <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esjohanss/8157153/hotspot.00-01/">http://cr.openjdk.java.net/~sjohanss/8157153/hotspot.00-01/</a>
<br>
<br>
Thanks, <br>
Stefan <br>
<br>
<blockquote type="cite"> <br>
/Mikael <br>
<br>
<blockquote type="cite"> <br>
Summary: <br>
The test sometimes fail due to running out of memory and the
reason is <br>
that the test expects to be able to fit 2 objects of the
size <br>
HeapRegionSize/2 into one heap region. This assumption is ok
in theory, <br>
but if any object at all is allocated between the two
allocations the <br>
second one will end up in a different region. The test
calculates how <br>
many regions it should fill to get 90% of the heap used and
then <br>
allocated twice this number of objects. In many cases this
will work <br>
since nothing else is going on, but in some cases some other
subsystem <br>
might start allocating objects, and this destroys the
assumptions <br>
leading to hitting an OOME. <br>
<br>
The fix simply adds a second check to the allocation loop to
stop as <br>
soon as the free memory drops below 10% of the heap, this
also requires <br>
some extra limiting of the indexes used. <br>
<br>
Testing: <br>
* Local testing as well as runs through RBT. <br>
<br>
Thanks, <br>
Stefan <br>
</blockquote>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>