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

[Dmitry Samersoff]

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.