Request for review: 7104173 sun/tools tests fail with debug build after 7012206

[David Holmes]

Well this is embarrassing. 7012206 changed the tests to always pass 
-XX:+UsePerfData. 7104173 changed the default of PrintVMOptions in a 
debug build for Embedded only. That means that all non-Embedded debug 
builds still fail all these tests. <sigh>

David

On 28/10/2011 12:45 AM, Paul Hohensee wrote:
> Looks good to me too. And, I'm an "official reviewer". :)
>
> Paul
>
> On 10/27/11 8:00 AM, Coleen Phillimore wrote:
>> This still seems fine to me (after reading the discussion and
>> 6605954). I don't know why someone wanted PrintVMOptions on in the
>> regular product, but it having this divergence for embedded is okay
>> with me.
>>
>> Coleen
>>
>> On 10/27/2011 6:47 AM, David Holmes wrote:
>>> On 27/10/2011 7:13 PM, Dmitry Samersoff wrote:
>>>>> I don't know the history of that option but maybe we should disable
>>>>> it for non-embedded builds too?
>>>>
>>>> +1
>>>>
>>>> I also would prefer to keep the same behavior for embedded/non-embedded
>>>> when possible.
>>>
>>> 6605954 was closed as "not a defect". So non-embedded has already
>>> expressed a desire to keep the current behaviour. Hence the
>>> embedded-only change.
>>>
>>> Thanks for the "reviews" ;-) I'll wait and see if I can get another
>>> formal review from runtime before pushing.
>>>
>>> David
>>>
>>>> -Dmitry
>>>>
>>>> On 2011-10-27 13:00, Christian Thalinger wrote:
>>>>>
>>>>> On Oct 27, 2011, at 10:43 AM, David Holmes wrote:
>>>>>
>>>>>> Hi Chris,
>>>>>>
>>>>>> On 27/10/2011 5:51 PM, Christian Thalinger wrote:
>>>>>>>
>>>>>>> On Oct 26, 2011, at 8:33 AM, David Holmes wrote:
>>>>>>>
>>>>>>>> webrev:
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~dholmes/7104173/webrev/
>>>>>>>>
>>>>>>>> A trivial change for our embedded code to aid in testing. Some
>>>>>>>> tests need to be passed -XX:+UsePerfData to make them work
>>>>>>>> (7012206). A debug build, by default, prints any -XX options
>>>>>>>> passed to it. Any such tests that parse the test output then
>>>>>>>> fail with a debug build because of this extra unexpected output.
>>>>>>>>
>>>>>>>> So for embedded only we simply turn PrintVMOptions off by
>>>>>>>> default for all builds.
>>>>>>>
>>>>>>> Why aren't you also passing -PrintVMOptions for the tests that
>>>>>>> need +UsePerfData?
>>>>>>
>>>>>> When 701226 was fixed I didn't realize there would be a need to
>>>>>> add -PrintVMOptions. Rather than go through and pollute all those
>>>>>> test scripts with yet-another-option I decided to deal with the
>>>>>> problem at the source - the VM. There really is no need to print
>>>>>> the options by default with a debug build. So for the embedded
>>>>>> build we turn it off. (See also 6605954)
>>>>>
>>>>> I don't know the history of that option but maybe we should disable
>>>>> it for non-embedded builds too? Sometimes the output of the VM
>>>>> options is useful but in that case I can add +PrintVMOptions manually.
>>>>>
>>>>> I don't have a strong opinion here I just don't like that embedded
>>>>> and non-embedded are diverging here. Anyway, the change is
>>>>> basically okay :-)
>>>>>
>>>>> -- Chris
>>>>>
>>>>>>
>>>>>> Cheers,
>>>>>> David
>>>>>>
>>>>>>
>>>>>>> -- Chris
>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> David
>>>>>>>
>>>>>
>>>>
>>>>