RFR: 8199736: Define WIN32_LEAN_AND_MEAN before including windows.h

[Robin Westberg]

Hi David,

Thanks for reviewing! I’ll delay progressing this one a bit until 8199619 is integrated.

Best regards,
Robin

> On 27 Mar 2018, at 02:57, David Holmes <david.holmes at oracle.com> wrote:
> 
> 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.
>>>>>