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

[Dmitry Samersoff]

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
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>
> 


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.