<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix"><br>
Hi Thomas,<br>
<br>
On 4/14/14 10:33 AM, Thomas Schatzl wrote:<br>
</div>
<blockquote cite="mid:1397464432.2708.6.camel@cirrus" type="cite">
<pre wrap="">Hi all,
On Wed, 2014-04-02 at 16:23 +0200, Thomas Schatzl wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Hi all,
can I have reviews for the following fix for a test case?
</pre>
</blockquote>
<pre wrap="">
Bengt found a race situation where the test fails when compilation is
running while the test is run.
Instead of trying to find a way to synchronize the test with the
compiler, I changed the code to allow multiple code root chunk free
lists - the test uses one separate from the global one, so there are no
more synchronization problems, and the original change to remember the
number of already allocated code root chunks is superfluous too.
CR:
<a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8038930">https://bugs.openjdk.java.net/browse/JDK-8038930</a>
Here is a new webrev:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~tschatzl/8038930/webrev.1/">http://cr.openjdk.java.net/~tschatzl/8038930/webrev.1/</a></pre>
</blockquote>
<br>
This looks good to me.<br>
<br>
I think I would have preferred passing exposing
G1CodeRootSet::_default_chunk_manager and explicitly passing that
when initializing HeapRegionRemSet::_code_roots rather than having
the special treatment of the default value NULL in the
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
G1CodeRootSet constructor. I'll leave it to you to decide.<br>
<br>
Otherwise it looks good.<br>
<br>
Thanks,<br>
Bengt<br>
<br>
<br>
<blockquote cite="mid:1397464432.2708.6.camel@cirrus" type="cite">
<pre wrap="">
Testing:
Failing test with -Xcomp, jprt
</pre>
<blockquote type="cite">
<pre wrap="">This test case does some basic code validation on recently introduced
code root memory management for G1 introduced recently (JDK-8035406).
It assumed that before running the test, no code root memory has been
handed out.
The problem is now, when running with -Xcomp, before the internal VM
tests (and this test) are run, it is likely that compiled code already
did some code root allocations.
So the test needs to be aware of that, and not assume that the number of
code root memory handed out is not zero.
This change fixes that assumption.
CR:
<a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8038930">https://bugs.openjdk.java.net/browse/JDK-8038930</a>
Webrev:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~tschatzl/8038930/webrev/">http://cr.openjdk.java.net/~tschatzl/8038930/webrev/</a>
Testing:
jprt, manual invocation of test case with -Xcomp
</pre>
</blockquote>
<pre wrap="">
Thanks,
Thomas
</pre>
</blockquote>
<br>
</body>
</html>