Request for Review (s) - 8133023: ParallelGCThreads is not calculated correctly

Jon Masamitsu jon.masamitsu at oracle.com
Tue Dec 8 21:12:21 UTC 2015



On 12/8/2015 11:37 AM, Kim Barrett wrote:
> On Dec 8, 2015, at 1:41 PM, Jon Masamitsu <jon.masamitsu at oracle.com> wrote:
>> Latest delta on the patch is at
>>
>> http://cr.openjdk.java.net/~jmasa/8133023/delta.06/
>>
>> It contains a name change from the original patch
>> vm_init_before_ergo() -> init_before_ergo() and some
>> additional comments.
>>
>> Full patch is at
>>
>> http://cr.openjdk.java.net/~jmasa/8133023/webrev.06/
> A couple minor things.  I don't need another webrev for these.  Looks
> good otherwise.
>
> ------------------------------------------------------------------------------
> src/share/vm/runtime/vm_version.hpp
>    59   // Called as part of the runtime services initialization
>    60   // called from the management module initialization (via init_globals())
>
> Need period at end of line 59, and line 60 should start capitalized.
My intent was that it be a single sentence.  I've added a "which is" to 
the end of the
line to make it more clear.

  59   // Called as part of the runtime services initialization which is
  60   // called from the management module initialization (via 
init_globals())

>
> ------------------------------------------------------------------------------
> src/share/vm/runtime/vm_version.hpp
>    72   static void early_initialize() { }
>    73   // Called to initialize VM variables needing initialization
>    74   // after command line parsing
>    75   static void init_before_ergo(void) {}
>
> Missing period at end of sentence.

Fixed.

>
> I'd prefer there be a blank line between lines 72 and 73.
>
> Maybe the description here should have a similar sentence to the
> preceding description for early_initialize, e.g. add
>
>    Platforms that need to specialize this define VM_Version::init_before_ergo().

I added it.

Thanks.

Jon

>
> ------------------------------------------------------------------------------
>
>



More information about the hotspot-dev mailing list