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