RFR: 8199736: Define WIN32_LEAN_AND_MEAN before including windows.h

Robin Westberg robin.westberg at oracle.com
Mon Apr 9 13:14:50 UTC 2018


Hi all,

For the record, here’s the final changeset after rebasing and fixing the conflict with 8199619.

Webrev: http://cr.openjdk.java.net/~rwestberg/8199736/webrev.03/ <http://cr.openjdk.java.net/~rwestberg/8199736/webrev.03/>

Best regards,
Robin

> On 3 Apr 2018, at 12:18, Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com> wrote:
> 
> On 2018-03-26 01:03, David Holmes 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?
> 
> Using WIN32_LEAN_AND_MEAN is good industry practice. We already do so for the JDK libraries. This trims down the number of files included by windows.h to a more manageble number. Several years ago, Microsoft did the (in my opinion) bad decision to let windows.h include a huge number of interfaces, unless this macro was defined. Using WIN32_LEAN_AND_MEAN and then explicitly including other API headers makes compilation faster, and the intention of the code clearer.
> 
> On top of this, due to naming conflicts, it is not possible at all to use winsock2.h unless you define WIN32_LEAN_AND_MEAN, windows.h will then pull in the old v1 of winsock.
> 
> /Magnus
> 
>> 
>> 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) ?
>> 
>> 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.
>> 
>> 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> wrote:
>>>> 
>>>>> On Mar 22, 2018, at 10:34 AM, Robin Westberg <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.
>>>> 
>>> 
> 



More information about the hotspot-dev mailing list