RFR: 8080106: Refactor setup of parallel GC threads

[Kim Barrett]

On May 20, 2015, at 7:50 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
> 
> Hi all,
> 
> Here are the fixes I made after the code review comments from Jon and Kim:
> 
> http://cr.openjdk.java.net/~stefank/8080110/webrev.01.delta/
> http://cr.openjdk.java.net/~stefank/8080110/webrev.01/

Looks good.

> 
> http://cr.openjdk.java.net/~stefank/8080112/webrev.01.delta/
> http://cr.openjdk.java.net/~stefank/8080112/webrev.01/

------------------------------------------------------------------------------
src/share/vm/gc/cms/parCardTableModRefBS.cpp 
  44   assert(n_threads <= (uint)ParallelGCThreads,

Why cast ParallelGCThreads?  Its type is uintx; I don't see a need for
a cast here.  Oh, I see.  n_threads used to be int and
ParallelGCThreads was being cast to int to avoid signed/unsigned
comparison warnings.  With n_threads being changed to uint, the cast
was updated accordingly, rather than being eliminated as no longer
needed.

Sorry about missing this in the earlier review pass.

------------------------------------------------------------------------------
src/share/vm/gc/cms/parCardTableModRefBS.cpp 
  45          err_msg("Error: n_threads: %u > ParallelGCThreads: %u", n_threads, (uint)ParallelGCThreads));

Why the "Error:" prefix in the message; seems rather redundent, and
not typical usage.  (I only found 9 similar occurrances of "Error:" in
assert messages in hotspot).  I noticed the immediately preceding
assert is one of them...

Also, I'd prefer use of UINTX_FORMAT, rather than casting
ParallelGCThreads.

> 
> http://cr.openjdk.java.net/~stefank/8080113/webrev.01.delta/
> http://cr.openjdk.java.net/~stefank/8080113/webrev.01/

Looks good.

> 
> Entire patch:
> http://cr.openjdk.java.net/~stefank/8080106/webrev.01/
> 
> Thanks,