Pls review 7116481 (S)

Paul Hohensee paul.hohensee at oracle.com
Tue Nov 29 13:48:53 PST 2011


Thanks for the prompt review.

On 11/29/11 4:35 PM, Jesper Wilhelmsson wrote:
> Looks good to me.
> A few nits...
>
> In the documentation of the flags in globals.hpp:367-435, why did you chose to add the parenthesis after the flag type? If it doesn't have a specific meaning that you want to stress I think it makes the text less readable.

I put them in as a reference to the macro that implements them.  They're 
already
lower case even though they begin a sentence.  I'll take the parens out, 
since at
least one person finds them confusing. :)

> Thanks for fixing the line break at globals.hpp:483-484. There are plenty more like that one in that macro ;-)

Yes.  I do a few formatting fixes whenever I change a file that I think 
needs it :),
but most reviewers don't want to be distracted by such changes so I keep 
them
to a minimum.

>
> I'm not a native english speaker, but to me the last s on line 3866 seems as a typo. "for JDK 6 and earliers". You did an unrelated change in that line, maybe you could remove the s as well while you're at it? Provided it is a typo of course.

Fixed.

Paul

> /Jesper
>
>
> 29 nov 2011 kl. 21:32 skrev Paul Hohensee<paul.hohensee at oracle.com>:
>
>> 7116481: Commercial features in Hotspot must be gated by a switch
>>
>> Commercial features (those for which Oracle charges for production use, though
>> not for development and/or evaluation) must be gated by a "master" -XX switch.
>> It works in the same manner as the existing UnlockDiagnosticVMOptions and
>> UnlockExperimentalVMOptions switches, so the new switch's name is
>> UnlockCommercialVMOptions.  A commercial() macro designates which
>> switches are gated by UnlockCommercialVMOptions.  I added a single
>> commercial() -XX flag named FlightRecorder, which does nothing, but
>> will eventually gate use of the Java Flight Recorder now under development.
>>
>> Webrev here:
>>
>> http://cr.openjdk.java.net/~phh/7116481/
>>
>> Tested from the command line.  Hotspot accepts this:
>>
>> -XX:+UnlockCommercialVMOptions -XX:+FlightRecorder
>>
>> and doesn't accept this
>>
>> -XX:+FlightRecorder
>>
>> Thanks,
>>
>> Paul
>>
>>


More information about the hotspot-dev mailing list