Hi John,<div><br></div><div>Comments inline:<br><br><div class="gmail_quote">On Thu, Jan 26, 2012 at 8:32 AM, John Coomes <span dir="ltr"><<a href="mailto:John.Coomes@oracle.com">John.Coomes@oracle.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">Bengt Rutisson (<a href="mailto:bengt.rutisson@oracle.com">bengt.rutisson@oracle.com</a>) wrote:<br>
><br>
> Hi John,<br>
><br>
> Looks good.<br>
><br>
> One minor comment:<br>
><br>
> I'd prefer the test:<br>
><br>
> 1045     if (!FLAG_IS_DEFAULT(UseAdaptiveSizePolicy)) {<br>
><br>
> to be:<br>
><br>
> 1045     if (FLAG_IS_CMDLINE(UseAdaptiveSizePolicy)) {<br>
><br>
> I think users are only interested in the warning if they actually had<br>
> the switch on the command line. If hotspot turns on the flag<br>
> ergonomically I think it is just confusing to customers to see the warning.<br>
<br>
</div>Thanks for the review.  I'll make that change; it's more future-proof.<br>
<div class="im"><br></div></blockquote><div>Just a nitpick: the FLAG_IS_CMDLINE doesn't cover VM arguments that were set from a config file (.hotspotrc), whose origin would have been CONFIG_FILE.</div><div><br></div>
<div>It's nicer if there's a FLAG_IS_USER_SET or something, that covers all cases where the argument might be set by a user instead of ergo.</div><div><br></div><div>- Kris</div><div> </div></div></div>