Review request: JDK-8003539 -- Minimal VM. VM doesn't react to -Dcom.sun.management and -XX:+ManagementServer

[Joe Provino]

On 11/18/2012 5:08 PM, David Holmes wrote:
> Hi Joe,
>
> I think we should have utilized UNSUPPORTED_OPTION 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 vm_exit_on_initialization 
> 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
>>> process_argument(). 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 INCLUDE_MANAGEMENT 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
>>>> INCLUDE_MANAGEMENT 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 INCLUDE_MANAGEMENT is false.
>>>>>>
>>>>>> Webrev is here:
>>>>>> http://cr.openjdk.java.net/~jprovino/8003539/webrev.00/
>>>>>>
>>>>>> thanks.
>>>>>>
>>>>>> joe
>>>>>
>>>
>>>