Review request: JDK-8003539 -- Minimal VM. VM doesn't react to -Dcom.sun.management and -XX:+ManagementServer (original) (raw)
Joe Provino joseph.provino at oracle.com
Sun Nov 18 14:36:52 PST 2012
- Previous message: Review request: JDK-8003539 -- Minimal VM. VM doesn't react to -Dcom.sun.management and -XX:+ManagementServer
- Next message: Review request: JDK-8003539 -- Minimal VM. VM doesn't react to -Dcom.sun.management and -XX:+ManagementServer
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
On 11/18/2012 5:08 PM, David Holmes wrote:
Hi Joe,
I think we should have utilized UNSUPPORTEDOPTION for those flags that have to be disabled and will generate a warning (eg all the GC warnings) - even if we have to augment the macro somewhat.
That sounds reasonable. Should I do that now or just make minimal changes?
Then we could have a second macro for the vmexitoninitialization case (though the distinction between a fatal error and a warning seems to be quite arbitrary :( ).
Yeah, it's not entirely clear when we warn or exit. I do recall a discussion about how we should warn if the only impact is potentially performance and exit if something is likely to fail. Customers may have scripts that they'd like to continue to work as long as missing functionality won't prevent them from working. Maybe we should revisit this.
Otherwise this change seems functionally correct, but the indentation appears wrong for the -Dcom.sun.management change.
Are you referring to line 2440? I looked at other calls to vm_exit_during_initialization and sometimes it's indented two spaces, other times four spaces. I agree they should all be done the same way. Is 2 spaces the standard for something like this?
thanks for the review.
joe
David On 19/11/2012 7:31 AM, Joe Provino wrote:
How does this look? http://cr.openjdk.java.net/~jprovino/8003539/webrev.01/ On 11/16/2012 4:13 PM, BILL PITTORE wrote: Where you had it is the right place, you just need to #if it better. The -XX: options get process by the code just after line 2788 by processargument(). That deals with all the globals defined in globals.hpp so it's not where you can special case one flag.
bill
On 11/16/2012 3:23 PM, Joe Provino wrote: Dmitri, wait, I think I see what you mean. If INCLUDEMANAGEMENT is set to true, then there's no code generated. I think the code that's there will work but perhaps there's a better place to check for this option when INCLUDEMANAGEMENT is false. I seem to recall there's a place were -XX: arguments are parsed But I don't see a separate method to do that. I must be looking in the wrong place. joe On 11/16/2012 3:14 PM, Dmitry Samersoff wrote: Joe, After you fix at 2788 regular hotspot start accepting do-nothing option -XX:+ManagementServer Is it intentional? -Dmitry
On 2012-11-16 23:24, Joe Provino wrote: This is a small change to one file -- arguments.cpp -- to print an error message and exit if INCLUDEMANAGEMENT is false. Webrev is here: http://cr.openjdk.java.net/~jprovino/8003539/webrev.00/ thanks. joe
- Previous message: Review request: JDK-8003539 -- Minimal VM. VM doesn't react to -Dcom.sun.management and -XX:+ManagementServer
- Next message: Review request: JDK-8003539 -- Minimal VM. VM doesn't react to -Dcom.sun.management and -XX:+ManagementServer
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]