RFR (XS): Run SPECjbb in headless mode in JPRT

Thu May 31 18:02:54 PDT 2012

Unless there are veto:s - can somebody help me sponsor the change?

Thanks,
Mikael

On 2012-05-30 06:16, Daniel D. Daugherty wrote:
> Personally, I'm finding it amazing that such a simple change to
> a JPRT config file is generating so many e-mails... I've seen
> complicated code float right on by with nary a whisper of a
> comment and yet this thread gets so many...
>
> "Amazing!" - Professor Bunsen Jude the Science Dude
>
> Dan - not enough coffee yet... :-)
>
> On 5/30/12 1:43 AM, Staffan Larsen wrote:
>> I agree that it should be the exception, but it should be allowed. I 
>> find it especially appropriate when the fix a is small (typically a 
>> one-liner) and when it may look out of place in the code and could 
>> use a little explanation.
>>
>> /Staffan
>>
>> On 30 maj 2012, at 09:32, David Holmes wrote:
>>
>>> On 30/05/2012 5:12 PM, Staffan Larsen wrote:
>>>> The changes look good.
>>>>
>>>> I'm in favor of having bug ids in the code. As Mikael says, it 
>>>> makes it much easier to find and understand the background of a 
>>>> fix. If you later change the code so that the fix isn't valid 
>>>> anymore, then the comment should of course be updated.
>>> That is not maintainable in general. You cannot annotate every line 
>>> of code with the bug fixes that have affected it (well you can but 
>>> the way to do that is through the sccs not comments! - and you need 
>>> the sccs to see the full changeset to understand the complete fix).
>>>
>>> Sometimes reference to a bug id is appropriate in a comment, but 
>>> this should be the exception not the norm.
>>>
>>> David
>>> -----
>>>
>>>> Thanks,
>>>> /Staffan
>>>>
>>>> On 30 maj 2012, at 05:39, Vladimir Kozlov wrote:
>>>>
>>>>> On 5/29/12 8:11 PM, David Holmes wrote:
>>>>>> On 30/05/2012 10:48 AM, Mikael Vidstedt wrote:
>>>>>>> On 2012-05-29 15:24, Vladimir Kozlov wrote:
>>>>>>>> We usually don't add Bug ID into code change. I think the comment
>>>>>>>> could be general: "Prevent popups during testing".
>>>>>>> There are many references to bug IDs in HotSpot and I'd argue 
>>>>>>> that it's
>>>>>>> extremely useful to have a link to the bug in the actual source 
>>>>>>> code
>>>>>>> since that allows me to more easily find more information about 
>>>>>>> why the
>>>>>>> fix was made, along with the potential discussion in the bug 
>>>>>>> tracker.
>>>>>> As this is a specific workaround (that might be removed in the 
>>>>>> future?) I think tagging it with the bug id is ok.
>>>>>>
>>>>>> But, historical baggage notwithstanding, generally we do not want 
>>>>>> to associate bug ids with code changes. It should be
>>>>>> the exception not the norm.
>>>>> Totally agree. We have 'hg log' and other tools if you want to see 
>>>>> bug ID associated with changes. And I think it is not workaround 
>>>>> but a fix which will stay. Why you want to enable popups for batch 
>>>>> jobs?
>>>>>
>>>>> The problem with bug IDs in a code is the code could be changed 
>>>>> later for an other bug so the comment will be not correct unless 
>>>>> you start to list all bugs IDs which is not realistic and wrong. 
>>>>> Or you will remove bug ID later, then why to have it at all.
>>>>>
>>>>> We have some Bug ID and comments which say "the next change is 
>>>>> done until such bug is fixed". But even such case should be the 
>>>>> exception.
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>>> Cheers,
>>>>>> David
>>>>>> -----
>>>>>>
>>>>>>> Mentioning the actual bug and the reason for doing the fix in the
>>>>>>> comment also serves as a warning for others that may feel 
>>>>>>> tempted to
>>>>>>> remove the line going forward.
>>>>>>>
>>>>>>> If it's ok with you I'd prefer to keep the comment the way it is?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Mikael
>>>>>>>
>>>>>>>> Vladimir
>>>>>>>>
>>>>>>>> Mikael Vidstedt wrote:
>>>>>>>>> New webrev below. The list of targets now matches the linux 
>>>>>>>>> x64 list
>>>>>>>>> (apart from the platform name that is).
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~mikael/7155453/webrev.02/
>>>>>>>>>
>>>>>>>>> Can I get a couple of Reviews and a sponsor?
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Mikael
>>>>>>>>>
>>>>>>>>> On 2012-05-23 17:27, Vladimir Kozlov wrote:
>>>>>>>>>> You mean missing "comma and backslash"?:
>>>>>>>>>>
>>>>>>>>>> ${jprt.my.macosx.x64}-{product|fastdebug}-c2-GCOld_ParOldGC, \
>>>>>>>>>>
>>>>>>>>>> Vladimir
>>>>>>>>>>
>>>>>>>>>> Mikael Vidstedt wrote:
>>>>>>>>>>> After addressing the missing backslash it now passes through 
>>>>>>>>>>> JPRT
>>>>>>>>>>> (on all platforms).
>>>>>>>>>>>
>>>>>>>>>>> Cheers,
>>>>>>>>>>> Mikael
>>>>>>>>>>>
>>>>>>>>>>> On 2012-05-22 17:20, Vladimir Kozlov wrote:
>>>>>>>>>>>> Did you run it through JPRT testing?
>>>>>>>>>>>>
>>>>>>>>>>>> Vladimir
>>>>>>>>>>>>
>>>>>>>>>>>> Mikael Vidstedt wrote:
>>>>>>>>>>>>> Thanks for the review. I updated based on your feedback.
>>>>>>>>>>>>>
>>>>>>>>>>>>> This new webrev also re-enables the JBB testing on OSX:
>>>>>>>>>>>>>
>>>>>>>>>>>>> http://cr.openjdk.java.net/~mikael/7155453/webrev.01
>>>>>>>>>>>>>
>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>> Mikael
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 2012-05-22 13:24, Daniel D. Daugherty wrote:
>>>>>>>>>>>>>> On 5/22/12 2:16 PM, Mikael Vidstedt wrote:
>>>>>>>>>>>>>>> All,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I'd like to get a couple of reviews of this change.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> We've seen problems with intermittent and hard to debug 
>>>>>>>>>>>>>>> popups
>>>>>>>>>>>>>>> when running SPECjbb on lights out OSX machines. To 
>>>>>>>>>>>>>>> prevent the
>>>>>>>>>>>>>>> popups from blocking the completion of the tests this 
>>>>>>>>>>>>>>> change
>>>>>>>>>>>>>>> makes SPECjbb run in headless mode in JPRT.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~mikael/7155453/webrev.00
>>>>>>>>>>>>>> Thumbs up.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> make/jprt.properties
>>>>>>>>>>>>>> typo: "popup:s" ->   "popups"
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The comment kind of implies that this is a MacOS X only 
>>>>>>>>>>>>>> change,
>>>>>>>>>>>>>> but it isn't. Maybe add one more line:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> # but the work-around is added to all platforms to be 
>>>>>>>>>>>>>> consistent
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Dan
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>