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

Robin Westberg robin.westberg at oracle.com
Mon Mar 26 15:01:10 UTC 2018

Hi David,

Thanks for taking a look!

On 26 Mar 2018, at 01:03, David Holmes <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:

/*

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/ <http://cr.openjdk.java.net/~rwestberg/8199736/webrev.02/> Updated webrev (incremental): http://cr.openjdk.java.net/~rwestberg/8199736/webrev.01-02/ <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> 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 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 build-dev mailing list