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

Kim Barrett kim.barrett at oracle.com
Wed Dec 9 08:24:34 UTC 2015


On Dec 8, 2015, at 4:12 PM, Jon Masamitsu <jon.masamitsu at oracle.com> wrote:
> 
> 
> 
> 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())

OK.

> 
>> 
>> ------------------------------------------------------------------------------
>> 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