RFR: 8199736: Define WIN32_LEAN_AND_MEAN before including windows.h (original) (raw)

Robin Westberg robin.westberg at oracle.com
Wed Mar 28 13:43:22 UTC 2018

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 WIN32LEANANDMEAN 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. ushort comes from types.h on non-Windows, is it simply missing on Windows (at least once we have WIN32LEANANDMEAN 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 uchar; typedef unsigned short ushort; typedef unsigned int uint; typedef unsigned long ulong; I noticed that one of these (uchar) is also defined in globalDefinitions.hpp so could perhaps define ushort 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 WIN32LEANANDMEAN [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#fasterbuildswithsmallerheaderfiles <https://msdn.microsoft.com/en-us/library/windows/desktop/aa383745(v=vs.85).aspx#fasterbuildswithsmallerheaderfiles> I think the addition of the WIN32LEANANDMEAN definition should be done through the build system, so that it applies everywhere.

More information about the hotspot-dev mailing list