request for review (m) - 7121618

[Tony Printezis]

Jon,

Thanks for doing this cleanup! Only some minor nits:


Is "...worker_id-th task queue" a valid expression? :-) Maybe "...task 
queue for worker_id"?

concurrentMark.cpp:

1701       gclog_or_tty->print("     Worker thread %d [%8.3f..%8.3f = %8.3f ms] "
1702                           "claimed %d regions (tot = %8.3f ms, max = %8.3f ms).\n",
1703                           worker_id, start, end, (end-start)*1000.0,

Now that worker_id is uint shouldn't "Worker thread %d..." be "Worker 
thread %u..."?


2921   int              _worker_id;

3024   int              _worker_id;

3056   int              _worker_id;

(and their uses)

3308 bool ConcurrentMark::do_yield_check(int worker_id) {

You don't want to turn these into uint's too?)


g1CollectorPolicy.cpp

2319     ParKnownGarbageHRClosure parKnownGarbageCl(_hrSorted, _chunk_size,
2320       worker_id);

worker_id should be aligned with _hrSorted.


genCollectedHeap.hpp

  422   void set_par_threads(uint t);
  423
  424

Maybe remove one of the empty lines?


referenceProcessor.hpp

  271   uint num_q()                             { return _num_q; }
  272   uint max_num_q()                         { return _max_num_q; }
  273   void set_active_mt_degree(uint v)         { _num_q = v; }

Please re-align { _num_q = v; } with the two lines above it.


workgroup.hpp

  332   FlexibleWorkGang(const char* name, uint workers,
  333                    bool are_GC_task_threads,
  334                    bool  are_ConcurrentGC_threads) :
  335     WorkGang(name, workers, are_GC_task_threads, are_ConcurrentGC_threads),
  336     _active_workers(UseDynamicNumberOfGCThreads ? 1U : ParallelGCThreads) {};

(not your fault) Why is there a semicolon at the end? Can you maybe 
remove it?

  377   Monitor* monitor()        { return&_monitor; }
  378   uint      n_workers()      { return _n_workers; }
  379   uint      n_completed()    { return _n_completed; }
  380   bool     should_reset()   { return _should_reset; }

Please re-align.

Tony

On 12/20/2011 01:34 PM, Jon Masamitsu wrote:
> I've updated the webrev for review comments
> (thanks, Bengt).
>
> http://cr.openjdk.java.net/~jmasa/7121618/webrev.01/
>
> On 12/19/11 11:33, Jon Masamitsu wrote:
>> This changes the type used for the number of GC threads to unsigned 
>> int from
>> mostly size_t and changes corresponding variables that take the 
>> number of GC
>> threads also to unsigned int.  Some variables names were also changed in
>> the process (principally "int i" in the work() method where "i" 
>> refers to
>> the thread to "uint worker_id").
>>
>> There are many changes but all are straightforward type or
>> name changes.
>>
>> 7121618: Change type of number of GC workers to unsigned int.
>>
>> http://cr.openjdk.java.net/~jmasa/7121618/webrev.00/
>>
>> Thanks.