<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<br>
<br>
<div class="moz-cite-prefix">On 2015-04-09 14:30, Evgeniya Stepanova
wrote:<br>
</div>
<blockquote cite="mid:552670DA.1020209@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html; charset=utf-8">
Hi,<br>
<br>
Please review fix for JDK-6407976.<br>
In some source files GC worker threads numbers are being stored as
<br>
signed values and same values in other classes are being stored as
unsigned <br>
values. Fix changed GC worker thread number to be uint.<br>
<br>
bug: <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-6407976">https://bugs.openjdk.java.net/browse/JDK-6407976</a><br>
webrev: <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eeistepan/6407976/webrev.00/">http://cr.openjdk.java.net/~eistepan/6407976/webrev.00/</a><br>
</blockquote>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~eistepan/6407976/webrev.00/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp.patch">http://cr.openjdk.java.net/~eistepan/6407976/webrev.00/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp.patch</a><br>
<br>
void G1CollectedHeap::reset_taskqueue_stats() {<br>
- const int n = workers()->total_workers();<br>
- for (int i = 0; i < n; ++i) {<br>
+ const uint n = workers()->total_workers();<br>
+ for (int i = 0; i < (int) n; ++i) {<br>
task_queue(i)->stats.reset();<br>
}<br>
}<br>
<br>
Why do you have this (int) cast here? Didn't you change the
task_queue function with:<br>
- RefToScanQueue *task_queue(int i) const;<br>
+ RefToScanQueue *task_queue(uint i) const;<br>
<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~eistepan/6407976/webrev.00/src/share/vm/gc_implementation/shared/adaptiveSizePolicy.cpp.patch">http://cr.openjdk.java.net/~eistepan/6407976/webrev.00/src/share/vm/gc_implementation/shared/adaptiveSizePolicy.cpp.patch</a><br>
<br>
You should keep the parameter alignments:<br>
<br>
-int AdaptiveSizePolicy::calc_active_workers(uintx total_workers,<br>
+uint AdaptiveSizePolicy::calc_active_workers(uintx total_workers,<br>
uintx active_workers,<br>
uintx
application_workers) {<br>
<br>
-int AdaptiveSizePolicy::calc_active_conc_workers(uintx
total_workers,<br>
+uint AdaptiveSizePolicy::calc_active_conc_workers(uintx
total_workers,<br>
uintx
active_workers,<br>
uintx
application_workers) {<br>
<br>
<br>
This goes for the other files as well.<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~eistepan/6407976/webrev.00/src/share/vm/runtime/vm_version.cpp.udiff.html">http://cr.openjdk.java.net/~eistepan/6407976/webrev.00/src/share/vm/runtime/vm_version.cpp.udiff.html</a><br>
<br>
As Igor mentioned, it would be good to only use uint:<br>
<br>
-int Abstract_VM_Version::_parallel_worker_threads = 0;<br>
+unsigned int Abstract_VM_Version::_parallel_worker_threads = 0;<br>
<br>
Thanks,<br>
StefanK<br>
<br>
<blockquote cite="mid:552670DA.1020209@oracle.com" type="cite"> <br>
tested by running all hotspot tests on all platforms<br>
<br>
<br>
Thanks,<br>
Jane<br>
<div class="moz-signature"><br>
</div>
</blockquote>
<br>
</body>
</html>