RFR(M): 6407976: GC worker number should be unsigned

Stefan Karlsson stefan.karlsson at oracle.com
Tue Apr 14 12:05:58 UTC 2015



On 2015-04-09 14:30, Evgeniya Stepanova wrote:
> Hi,
>
> Please review fix for  JDK-6407976.
> In some source files GC worker threads numbers are being stored as
> signed values and same values in other classes are being stored as 
> unsigned
> values. Fix changed GC worker thread number to be uint.
>
> bug: https://bugs.openjdk.java.net/browse/JDK-6407976
> webrev: http://cr.openjdk.java.net/~eistepan/6407976/webrev.00/

http://cr.openjdk.java.net/~eistepan/6407976/webrev.00/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp.patch

  void G1CollectedHeap::reset_taskqueue_stats() {
-  const int n = workers()->total_workers();
-  for (int i = 0; i < n; ++i) {
+  const uint n = workers()->total_workers();
+  for (int i = 0; i < (int) n; ++i) {
      task_queue(i)->stats.reset();
    }
  }

Why do you have this (int) cast here? Didn't you change the task_queue 
function with:
-  RefToScanQueue *task_queue(int i) const;
+  RefToScanQueue *task_queue(uint i) const;


http://cr.openjdk.java.net/~eistepan/6407976/webrev.00/src/share/vm/gc_implementation/shared/adaptiveSizePolicy.cpp.patch

You should keep the parameter alignments:

-int AdaptiveSizePolicy::calc_active_workers(uintx total_workers,
+uint AdaptiveSizePolicy::calc_active_workers(uintx total_workers,
                                              uintx active_workers,
                                              uintx application_workers) {

-int AdaptiveSizePolicy::calc_active_conc_workers(uintx total_workers,
+uint AdaptiveSizePolicy::calc_active_conc_workers(uintx total_workers,
                                                   uintx active_workers,
                                                   uintx 
application_workers) {


This goes for the other files as well.

http://cr.openjdk.java.net/~eistepan/6407976/webrev.00/src/share/vm/runtime/vm_version.cpp.udiff.html

As Igor mentioned, it would be good to only use uint:

-int Abstract_VM_Version::_parallel_worker_threads = 0;
+unsigned int Abstract_VM_Version::_parallel_worker_threads = 0;

Thanks,
StefanK

>
> tested by running all hotspot tests on all platforms
>
>
> Thanks,
> Jane
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/attachments/20150414/72049d76/attachment.html>


More information about the hotspot-gc-dev mailing list