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

[Magnus Ihse Bursie]

On 2015-09-21 14:19, Magnus Ihse Bursie wrote:
> Inte för att stressa dig eller så, men du glömmer inte bort den här 
> va? Den är en prereq för att kunna få in Tonga, och jag håller på och 
> knyter ihop Tonga-säcken i övrigt.

I apologize for this. It was supposed to be a private message to Erik.

/Magnus

>
> /Magnus
>
> On 2015-09-16 11:37, Magnus Ihse Bursie wrote:
>> 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
>>
>