RFR: JDK-8136385: Various build speed improvements for windows

[Magnus Ihse Bursie]

On 2015-09-11 16:46, Erik Joelsson wrote:
> Hello,
>
> In the build-infra project, I have made various minor build speed 
> improvements for Windows. These mostly affect incremental build 
> performance, but in certain cases normal builds are also greatly 
> affected. I find these worth committing to JDK 9 separately.
>
> List of improvements:
>
> * Rewrote DependOnVariable to use "-include" instead of $(shell $(CAT) 
> ...) to read the old variable value from the last make invocation. 
> This has a pretty big impact on incremental build performance on 
> Windows. Also started using the file function in make when available 
> (in make 4.0) instead of $(shell $(PRINTF) ...) when writing these files.
>
> * Implemented ListPathsSafely using the file function when available. 
> Since we require make 4.0 in cygwin, this will usually be the case 
> when it matters.
>
> * Reduced the number of shell execution in the compiler and link 
> recipes by joining them together with "; \". When compiling a lot of 
> native code, this tends to get quite expensive.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8136385
> Webrev: http://cr.openjdk.java.net/~erikj/8136385/webrev.01/

As a general comment, I've experimented a bit with using .ONESHELL, 
which will automatically concatenate separate lines in a recipe and 
execute them by a single shell. When it works, it improves performance 
significantly, especially in Windows. Having it enabled all over the 
board would also let us avoid those "&& \" constructs.

However, a lot of recipes needs to be updated to work properly with 
.ONESHELL. Also, it was also introduced in GNU Make 3.82 so we would 
need to bump our lowest supported platform number. Nevertheless, I think 
it is the proper way forward, but it will take some time. So changes 
such as this will be needed in the meantime.

I have a few questions:

In JavaCompilation.gmk, you replace $1_GREP_INCLUDE_OUTPUT := with 
$1_GREP_INCLUDE_OUTPUT =. Any specific reason to drop the : or just a 
typo? (And so also for $1_GREP_EXCLUDE_OUTPUT)

The ListPathsSafely function is really horrible and hard to read. :-/ 
(I'm not blaming you; the version using "file"  is excellent). I have a 
hard time figuring out that the legacy version (without "file") is still 
correct. Maybe we should have a test for it?

The indentation on DependOnVariableHelper looks weird. It might be the 
patch but I'm not sure.

In NativeCompilation, I'm trying to figure out how the "ifneq 
($$($1_$2_DEP),)" is supposed to work. This is old code and I wouldn't 
have reacted to it if it were not for the fact that you removed this 
checked for the microsoft toolchain path. As I can see, the $1_$2_DEP 
variable will be empty for .s files. But if we're compiling .s files in 
solstudio, we'll still end up calling "$$($1_$2_COMP) $$($1_$2_FLAGS) 
$$($1_$2_DEP_FLAG) $$($1_$2_DEP) ...". How could that possibly work? And 
what happens for the microsoft case with your patch? Now we're going to 
do the "$(SED) $(DEPENDENCY_TARGET_SED_PATTERN) $$($1_$2_DEP) > 
$$($1_$2_DEP_TARGETS)" operation, which we previously didn't do. Do we 
actually compile any .s files? I can't understand how that would work. 
*checking* No, we don't. In the jdk project, there's just a single .s 
file 
(./jdk/src/java.desktop/unix/native/libawt/awt/medialib/mlib_v_ImageCopy_blk.s) 
and it's excluded from compilation. On the other hand, there *are* .s 
files in the Hotspot project which we will need to be able to handle at 
some point. Urgh. Messy. You'll have to decide if you want to clean this up.

/Magnus

>
> /Erik