RFR (S): 8022452: Hotspot needs to know about Windows 8.1 and Windows Server 2012 R2

[Mikael Vidstedt]

Dmitry/Coleen/everybody else - thanks for the feedback!

Cheers,
Mikael

On 2013-08-09 09:33, Dmitry Samersoff wrote:
> Mikael,
>
> Fill free to continue.
>
> Changes looks OK for me.
>
> -Dmitry
>
> On 2013-08-09 20:18, Mikael Vidstedt wrote:
>> The Oracle JDK does not support the older Windows versions in JDK 7.
>> However, in an attempt to keep this change small I will keep those
>> checks in there and queue up a future cleanup task.
>>
>> Unless the comment is annoying you all too much I'll keep it in there too.
>>
>> Cheers,
>> Mikael
>>
>> On 2013-08-09 04:15, Dmitry Samersoff wrote:
>>> Mikael,
>>>
>>> Does we still support Win 95/98/Me ?
>>>
>>> PS: Comments seems redundant to me
>>>
>>> 1660     // find out whether we are running on 64 bit processor or not.
>>>
>>> -Dmitry
>>>
>>> On 2013-08-09 03:59, Coleen Phillimore wrote:
>>>> Mikael,
>>>>
>>>> I think this looks great.  It's a lot cleaner (more red than blue
>>>> changes).
>>>>
>>>> thanks for humoring me,
>>>> Coleen
>>>>
>>>> On 08/08/2013 06:25 PM, Mikael Vidstedt wrote:
>>>>> Coleen,
>>>>>
>>>>> After having tried numerous different versions I ended up with this:
>>>>>
>>>>> http://cr.openjdk.java.net/~mikael/webrevs/8022452.3/webrev
>>>>>
>>>>> Summary of the changes:
>>>>>
>>>>> * The GetNativeSystemInfo call has been moved up before the switch,
>>>>> but is only called if the os version is recent enough, and  we
>>>>> actually care about the si.wProcessorArchitecture field.
>>>>>
>>>>> * The true/else branches in the if-diamond have been switch to avoid
>>>>> the negation of the condition.
>>>>>
>>>>> * While at it I added support for recognizing "Windows Server 2008 R2"
>>>>> (6.1 non-workstation) which we apparently haven't recognized earlier,
>>>>> but which is something the JDK code recognizes [1].
>>>>>
>>>>> * The " , 64 bit" has been move out below the switch, but is only
>>>>> relevant/tested for on recent enough Windows versions, just like the
>>>>> old code did.
>>>>>
>>>>> Feedback much appreciated! I'm also investigating if it would be
>>>>> possible to unify/share the code with the java_props_md.c logic [1] to
>>>>> avoid the duplication, but for now this may have to do. In addition to
>>>>> that the code can be cleaned up further if we only support VS2010+,
>>>>> which I believe is de facto since JDK 7 anyway; I will investigate
>>>>> that as a separate improvement.
>>>>>
>>>>> Thanks,
>>>>> Mikael
>>>>>
>>>>> [1]
>>>>> http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/e057cddf0d6c/src/windows/native/java/lang/java_props_md.c
>>>>>
>>>>> (line 451).
>>>>>
>>>>> On 2013-08-07 10:56, Coleen Phillimore wrote:
>>>>>> Mikael,
>>>>>>
>>>>>> I do like your new version better but because you changed so much, it
>>>>>> should be verified on all versions of windows.   Which you might not
>>>>>> want to do.   My suggestion to make this code nicer (fix case stmt
>>>>>> and outline common code) was really simple and I think can be
>>>>>> verified by code inspection, which might be less work and easier to
>>>>>> review.
>>>>>>
>>>>>> Coleen
>>>>>>
>>>>>> On 8/7/2013 12:28 PM, Mikael Vidstedt wrote:
>>>>>>> Coleen,
>>>>>>>
>>>>>>> Funny you should mention that, and since I fully agree with you I
>>>>>>> actually did prepare another version of the patch which is heavily
>>>>>>> inspired by the java_props_md.c implementation:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~mikael/webrevs/winvercleanup2/webrev/
>>>>>>>
>>>>>>> I personally like this version better, but the one thing that held
>>>>>>> me back was the fact that I will basically want to dig up machines
>>>>>>> with all the different (supported) Windows versions and verify that
>>>>>>> the code is still correct in all cases. If you think this is the way
>>>>>>> to go I more than willing to will do so. Let me know what you think
>>>>>>> about the new version of the patch!
>>>>>>>
>>>>>>> There's no real urgency here - I'd rather do it correct once than...
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Mikael
>>>>>>>
>>>>>>>
>>>>>>> On 2013-08-07 06:39, Coleen Phillimore wrote:
>>>>>>>> It looks like the code under the case for all these releases,
>>>>>>>> should be different case statements for each, rather than these
>>>>>>>> cascading 'if' statements.  And make 1668-1673 a new function
>>>>>>>> returning si.   So if we have a new release, in general it will
>>>>>>>> work because the default will print out something.   But a better
>>>>>>>> default would be lines 1712-1717 which are dead code now.
>>>>>>>>
>>>>>>>> Sorry, I know it's simple and urgent but this seems to be getting
>>>>>>>> increasingly ugly.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Coleen
>>>>>>>>
>>>>>>>> On 8/6/2013 7:36 PM, Mikael Vidstedt wrote:
>>>>>>>>> Please review the following change:
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~mikael/webrevs/8022452/webrev
>>>>>>>>>
>>>>>>>>> The patch adds support to os_windows.cpp for recognizing the
>>>>>>>>> upcoming Windows versions: Windows 8.1 and Windows Server 2012 R2.
>>>>>>>>>
>>>>>>>>> The changed is based on the corresponding code on the JDK
>>>>>>>>> side[1][2][3].
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Mikael
>>>>>>>>>
>>>>>>>>> [1] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8020191
>>>>>>>>> [2] http://hg.openjdk.java.net/jdk8/tl/jdk/rev/40221b09812f
>>>>>>>>> [3]
>>>>>>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-July/019463.html
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>