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