RFR: 8199736: Define WIN32_LEAN_AND_MEAN before including windows.h

[David Holmes]

Looks good to me.

Thanks,
David

On 27/03/2018 1:01 AM, Robin Westberg wrote:
> Hi David,
> 
> Thanks for taking a look!
> 
>> On 26 Mar 2018, at 01:03, David Holmes <david.holmes at oracle.com 
>> <mailto:david.holmes at oracle.com>> wrote:
>>
>> Hi Robin,
>>
>> On 23/03/2018 10:37 PM, Robin Westberg wrote:
>>> Hi Kim & Erik,
>>> Certainly makes sense to define it from the build system, I’ve 
>>> updated the patch accordingly:
>>> Full: http://cr.openjdk.java.net/~rwestberg/8199736/webrev.01/ 
>>> <http://cr.openjdk.java.net/~rwestberg/8199736/webrev.01/>
>>> Incremental: 
>>> http://cr.openjdk.java.net/~rwestberg/8199736/webrev.00-01/ 
>>> <http://cr.openjdk.java.net/~rwestberg/8199736/webrev.00-01/>
>>
>> I'm a little unclear on the hotspot changes. If we define 
>> WIN32_LEAN_AND_MEAN then certain APIs like sockets are excluded from 
>> windows.h so we then have to include the specific header files like 
>> winsock2.h - is that right?
> 
> Yep that’s correct, headers like winsock, dde, ole, shellapi and a few 
> other uncommon ones are no longer included from windows.h when this is 
> defined.
> 
>> src/hotspot/share/interpreter/bytecodes.cpp
>>
>> I'm curious about this change. u_short comes from types.h on 
>> non-Windows, is it simply missing on Windows (at least once we have 
>> WIN32_LEAN_AND_MEAN defined) ?
> 
> Yeah, on Windows these comes from winsock(2).h:
> 
> /*
>   * Basic system type definitions, taken from the BSD file sys/types.h.
>   */
> typedef unsigned char   u_char;
> typedef unsigned short  u_short;
> typedef unsigned int    u_int;
> typedef unsigned long   u_long;
> 
> I noticed that one of these (u_char) is also defined in 
> globalDefinitions.hpp so could perhaps define u_short there, or include 
> winsock2.h globally again. But since it was only used in a single place 
> in the existing code it seemed simple enough to just expand the typedef 
> there.
> 
>> src/hotspot/share/utilities/ostream.cpp
>>
>> 1029 #endif
>> 1030 #if defined(_WINDOWS)
>>
>> Using elif could be marginally faster given the two sets of conditions 
>> are mutually exclusive.
> 
> Good point, will change it.
> 
> I also had to move the flag definition to adapt to the latest changes in 
> the hs repo, cc’ing build-dev again to make sure I got it right.
> 
> Updated webrev (full): 
> http://cr.openjdk.java.net/~rwestberg/8199736/webrev.02/
> Updated webrev (incremental): 
> http://cr.openjdk.java.net/~rwestberg/8199736/webrev.01-02/
> 
> Best regards,
> Robin
> 
>>
>> Thanks,
>> David
>>
>>> (Not quite sure if the definition belongs where I put it or a bit 
>>> later where most other windows-specific JVM flags are defined, but 
>>> seemed reasonable to put it close to where it is defined for the JDK 
>>> libraries).
>>> Best regards,
>>> Robin
>>>> On 22 Mar 2018, at 16:52, Kim Barrett <kim.barrett at oracle.com 
>>>> <mailto:kim.barrett at oracle.com>> wrote:
>>>>
>>>>> On Mar 22, 2018, at 10:34 AM, Robin Westberg 
>>>>> <robin.westberg at oracle.com <mailto:robin.westberg at oracle.com>> wrote:
>>>>>
>>>>> Hi all,
>>>>>
>>>>> Please review the following change that defines WIN32_LEAN_AND_MEAN 
>>>>> [1] before including windows.h. This marginally improves build 
>>>>> times, and makes it possible to include winsock2.h.
>>>>>
>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8199736 
>>>>> <https://bugs.openjdk.java.net/browse/JDK-8199736>
>>>>> Webrev: http://cr.openjdk.java.net/~rwestberg/8199736/webrev.00/ 
>>>>> <http://cr.openjdk.java.net/~rwestberg/8199736/webrev.00/>
>>>>> Testing: hs-tier1
>>>>>
>>>>> Best regards,
>>>>> Robin
>>>>>
>>>>> [1] 
>>>>> https://msdn.microsoft.com/en-us/library/windows/desktop/aa383745%28v=vs.85%29.aspx#faster_builds_with_smaller_header_files 
>>>>> <https://msdn.microsoft.com/en-us/library/windows/desktop/aa383745(v=vs.85).aspx#faster_builds_with_smaller_header_files>
>>>>
>>>> I think the addition of the WIN32_LEAN_AND_MEAN definition should be 
>>>> done through the build
>>>> system, so that it applies everywhere.
>>>>
>