<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Derek,<br>
<br>
Fix and test look good.<br>
<br>
Reviewed.<br>
<br>
Jon<br>
<br>
<div class="moz-cite-prefix">On 04/28/2015 06:35 AM, Derek White
wrote:<br>
</div>
<blockquote cite="mid:553F8C88.7060902@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
<div class="moz-cite-prefix">Thanks Bengt!<br>
<br>
On 4/28/15 4:43 AM, Bengt Rutisson wrote:<br>
</div>
<blockquote cite="mid:553F4838.5010403@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 28/04/15 00:14, Derek White
wrote:<br>
</div>
<blockquote cite="mid:553EB4A9.80605@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">3rd webrev:<br>
<br>
<tt>Please review this fix for: <br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8076995">https://bugs.openjdk.java.net/browse/JDK-8076995</a></tt><br>
<pre wrap="">Webrev:
<a href="http://cr.openjdk.java.net/%7Edrwhite/8076995/webrev.01/" moz-do-not-send="true" class="moz-txt-link-freetext">http://cr.openjdk.java.net/~drwhite/8076995/webrev.02/</a></pre>
</div>
</blockquote>
<br>
The link above actually goes to webrev.01, but going to
webrev.02 works and that change looks good to me. Reviewed.<br>
<blockquote cite="mid:553EB4A9.80605@oracle.com" type="cite">
<div class="moz-cite-prefix"> <br>
Changes:<br>
<br>
- Updated TEST.groups to only run this test if G1, CMS, and
Parallel GCs are enabled. Removed code from webrev.01 that
also updated other GC tests. This is now tracked as a
separate bug <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8078673">https://bugs.openjdk.java.net/browse/JDK-8078673</a><br>
</div>
</blockquote>
<br>
Thanks for creating this bug to track the other tests that need
to be updated! Can you add a comment to JDK-8078673 that lists
the tests that you have identified as problematic?<br>
<br>
Thanks,<br>
Bengt<br>
<br>
<blockquote cite="mid:553EB4A9.80605@oracle.com" type="cite">
<div class="moz-cite-prefix"> <br>
FYI also created separate bug to track the processor count
issues: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8078703">https://bugs.openjdk.java.net/browse/JDK-8078703</a><br>
<br>
Thanks!<br>
<br>
- Derek<br>
<br>
On 4/27/15 4:04 AM, Bengt Rutisson wrote:<br>
</div>
<blockquote cite="mid:553DED8E.7070706@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<br>
Hi Derek,<br>
<br>
<div class="moz-cite-prefix">On 2015-04-23 23:55, Derek
White wrote:<br>
</div>
<blockquote cite="mid:55396A3F.8050402@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">2nd webrev:<br>
<br>
<tt>Please review this fix for: <br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8076995">https://bugs.openjdk.java.net/browse/JDK-8076995</a></tt><br>
<pre wrap="">Webrev:
<a href="http://cr.openjdk.java.net/%7Edrwhite/8076995/webrev.01/" moz-do-not-send="true" class="moz-txt-link-freetext">http://cr.openjdk.java.net/~drwhite/8076995/webrev.01/</a>
</pre>
<br>
Changes:<br>
<br>
- Updated TEST.groups to only run this test if G1, CMS,
and Parallel GCs are enabled.<br>
</div>
</blockquote>
<br>
OK.<br>
<br>
<blockquote cite="mid:55396A3F.8050402@oracle.com"
type="cite">
<div class="moz-cite-prefix"> - Also searched for similar
GC tests that specify a GC to use and added to
TESTS.groups:<br>
<ul>
<li>gc/TestSmallHeap.java</li>
<li>gc/logging/TestGCId.java</li>
<li>gc/TestCardTablePageCommits.java</li>
<li>gc/arguments/TestParallelHeapSizeFlags.java</li>
</ul>
</div>
</blockquote>
<br>
I think I would prefer this as a separate change. I'm not
very good with TEST.groups, but it looks like your suggested
changes are correct. However, I would prefer the get more
attention to that change and thus it would be better to send
it out as a separate review.<br>
<br>
Thanks,<br>
Bengt<br>
<br>
<blockquote cite="mid:55396A3F.8050402@oracle.com"
type="cite">
<div class="moz-cite-prefix">
<ul>
</ul>
- Responded to comments below.<br>
<br>
Did jprt run. Saw timeout, not sure if real or if it's
one of those "embedded tests don't quite fit" errors.<br>
Fail/kill comment: Targets failed. Target
linux_armvfpsflt_2.6-productEmb-c2-hotspot_servertest
timedout. <br>
<br>
- Derek<br>
<br>
On 4/21/15 1:23 PM, Jon Masamitsu wrote:<br>
</div>
<blockquote cite="mid:553687A0.8090802@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
Derek,<br>
<br>
Thanks for fixing this.<br>
<br>
Fix looks good.<br>
<br>
What do you think about always making
testDynamicNumberOfGCThread()<br>
check for the uniprocessor case (as opposed to passing
in a flag to explicitly<br>
check it)? <br>
<br>
ForceDynamicNumberOfGCThreads is a diagnostic flag<br>
<br>
diagnostic(bool, ForceDynamicNumberOfGCThreads,
false, \<br>
"Force dynamic selection of the number of
" \<br>
"parallel threads parallel gc will use to aid
debugging") \<br>
<br>
so I think you need +UnlockDiagnosticVMOptions.<br>
<br>
Jon<br>
<br>
<div class="moz-cite-prefix">On 04/21/2015 06:53 AM,
Derek White wrote:<br>
</div>
<blockquote cite="mid:5536563F.4020806@oracle.com"
type="cite">
<meta http-equiv="content-type" content="text/html;
charset=utf-8">
<tt>Hi All,</tt><tt><br>
</tt><tt><br>
</tt><tt>Please review this fix for: <br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8076995">https://bugs.openjdk.java.net/browse/JDK-8076995</a></tt><br>
<pre wrap="">Webrev:
<a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/%7Edrwhite/8076995/webrev.00/">http://cr.openjdk.java.net/~drwhite/8076995/webrev.00/</a>
Summary:
Part 1 is a test bug that tries to run G1 on embedded SE builds. Not changed by this webrev.
Part two is assertion failure that is being fixed by this webrev.
This is a fix for bug that triggered an assert when running CMS on very
small machines - 1 core x86, or 1-4 core ARM. This may seem unlikely but
can easily happen when running virtual instances.
Failure stack traces also show bug crashing printing a stack trace, but this is being tracked in another bug.
Thanks,
- Derek
</pre>
<tt><br>
</tt><tt><br>
</tt><tt><br>
</tt>
<meta http-equiv="content-type" content="text/html;
charset=utf-8">
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>