Re: Stop using precompiled headers for Linux?

[Erik Joelsson]

Below are the corresponding numbers from a Mac, (Mac Pro (Late 2013), 
3.7 GHz, Quad-Core Intel Xeon E5, 16 GB). To be clear, the -npch is 
without precompiled headers. Here we see a slight degradation when 
disabling on both user time and wall clock time. My guess is that the 
user time increase is about the same, but because of a lower cpu count, 
the extra load is not as easily covered.


These tests were run with just building hotspot. This means that the 
precompiled header is generated alone on one core while nothing else is 
happening, which would explain this degradation in build speed. If we 
were instead building the whole product, we would see a better 
correlation between user and real time.


Given the very small benefit here, it could make sense to disable 
precompiled headers by default for Linux and Mac, just as we did with 
ccache.


I do know that the benefit is huge on Windows though, so we cannot 
remove the feature completely. Any other comments?


/Erik

macosx-x64
real     4m13.658s
user     27m17.595s
sys     2m11.306s

macosx-x64-npch
real     4m27.823s
user     30m0.434s
sys     2m18.669s

macosx-x64-debug
real     5m21.032s
user     35m57.347s
sys     2m20.588s

macosx-x64-debug-npch
real     5m33.728s
user     38m10.311s
sys     2m27.587s

macosx-x64-slowdebug
real     3m54.439s
user     25m32.197s
sys     2m8.750s

macosx-x64-slowdebug-npch
real     4m11.987s
user     27m59.857s
sys     2m18.093s


On 2018-10-30 14:00, Erik Joelsson wrote:

Hello,

On 2018-10-30 13:17, Aleksey Shipilev wrote:

On 10/30/2018 06:26 PM, Ioi Lam wrote:

Is there any advantage of using precompiled headers on Linux?
I have measured it recently on shenandoah repositories, and 
fastdebug/release build times have not
improved with or without PCH. Actually, it gets worse when you touch 
a single header that is in PCH
list, and you end up recompiling the entire Hotspot. I would be in 
favor of disabling it by default.
I just did a measurement on my local workstation (2x8 cores x2 ht 
Ubuntu 18.04 using Oracle devkit GCC 7.3.0). I ran "time make hotspot" 
with clean build directories.


linux-x64:
real    4m6.657s
user    61m23.090s
sys    6m24.477s

linux-x64-npch
real    3m41.130s
user    66m11.824s
sys    4m19.224s

linux-x64-debug
real    4m47.117s
user    75m53.740s
sys    8m21.408s

linux-x64-debug-npch
real    4m42.877s
user    84m30.764s
sys    4m54.666s

linux-x64-slowdebug
real    3m54.564s
user    44m2.828s
sys    6m22.785s

linux-x64-slowdebug-npch
real    3m23.092s
user    55m3.142s
sys    4m10.172s

These numbers support your claim. Wall clock time is actually 
increased with PCH enabled, but total user time is decreased. Does not 
seem worth it to me.

It's on by default and we keep having
breakage where someone would forget to add #include. The latest 
instance is JDK-8213148.
Yes, we catch most of these breakages in CIs. Which tells me adding 
it to jdk-submit would cover

most of the breakage during pre-integration testing.
jdk-submit is currently running what we call "tier1". We do have 
builds of Linux slowdebug with precompiled headers disabled in tier2. 
We also build solaris-sparcv9 in tier1 which does not support 
precompiled headers at all, so to not be caught in jdk-submit you 
would have to be in Linux specific code. The example bug does not seem 
to be that. Mach5/jdk-submit was down over the weekend and yesterday 
so my suspicion is the offending code in this case was never tested.


That said, given that we get practically no benefit from PCH on 
Linux/GCC, we should probably just turn it off by default for Linux 
and/or GCC. I think we need to investigate Macos as well here.


/Erik

-Aleksey

Re: Stop using precompiled headers for Linux?

[Erik Joelsson]

Hello,

On 2018-10-30 13:17, Aleksey Shipilev wrote:

On 10/30/2018 06:26 PM, Ioi Lam wrote:

Is there any advantage of using precompiled headers on Linux?

I have measured it recently on shenandoah repositories, and fastdebug/release 
build times have not
improved with or without PCH. Actually, it gets worse when you touch a single 
header that is in PCH
list, and you end up recompiling the entire Hotspot. I would be in favor of 
disabling it by default.
I just did a measurement on my local workstation (2x8 cores x2 ht Ubuntu 
18.04 using Oracle devkit GCC 7.3.0). I ran "time make hotspot" with 
clean build directories.


linux-x64:
real    4m6.657s
user    61m23.090s
sys    6m24.477s

linux-x64-npch
real    3m41.130s
user    66m11.824s
sys    4m19.224s

linux-x64-debug
real    4m47.117s
user    75m53.740s
sys    8m21.408s

linux-x64-debug-npch
real    4m42.877s
user    84m30.764s
sys    4m54.666s

linux-x64-slowdebug
real    3m54.564s
user    44m2.828s
sys    6m22.785s

linux-x64-slowdebug-npch
real    3m23.092s
user    55m3.142s
sys    4m10.172s

These numbers support your claim. Wall clock time is actually increased 
with PCH enabled, but total user time is decreased. Does not seem worth 
it to me.

It's on by default and we keep having
breakage where someone would forget to add #include. The latest instance is 
JDK-8213148.

Yes, we catch most of these breakages in CIs. Which tells me adding it to 
jdk-submit would cover
most of the breakage during pre-integration testing.
jdk-submit is currently running what we call "tier1". We do have builds 
of Linux slowdebug with precompiled headers disabled in tier2. We also 
build solaris-sparcv9 in tier1 which does not support precompiled 
headers at all, so to not be caught in jdk-submit you would have to be 
in Linux specific code. The example bug does not seem to be that. 
Mach5/jdk-submit was down over the weekend and yesterday so my suspicion 
is the offending code in this case was never tested.


That said, given that we get practically no benefit from PCH on 
Linux/GCC, we should probably just turn it off by default for Linux 
and/or GCC. I think we need to investigate Macos as well here.


/Erik

-Aleksey

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

[Andy Herrick]

On 10/30/2018 10:11 AM, Andy Herrick wrote:



On 10/24/2018 10:22 AM, Alan Bateman wrote:

On 23/10/2018 16:49, Andy Herrick wrote:
This patch implements the Java Packager Tool (jpackager) as 
described in JEP 343: Packaging Tool 



jpackager is a simple packaging tool, based on the JavaFX 
|javapackager| tool, that:


 * Supports native packaging formats to give the end user a natural
   installation experience. These formats include |msi| and |exe| on
   Windows, |pkg| and |dmg| on MacOS, and |deb| and |rpm| on Linux.
 * Allows launch-time parameters to be specified at packaging time.
 * Can be invoked directly, from the command line, or programmatically,
   via the |ToolProvider| API.

Webrev:

http://cr.openjdk.java.net/~herrick/8212780/webrev.01/


cc'ing build-dev as it's important to get it reviewed there.

What is the plan for tests to go with this tool? I see there is one 
test in the webrev to do some argument validation but I don't see 
anything else right now.
We plan to incorporate the initial feedback from this review, and 
include an initial set of automated tests in a refresh sometime next 
week.

We will continue to develop and automate tests for future updates.


What is the status of the JNLPConverter tool? I see it is included as 
a "demo" but maybe it would be better to host somewhere else as this 
is for developers migrating Java Web Start applications.

Our current plan is to deliver it only as a demo.
After further discussion, we have decided to remove the JNLPConversion 
tool , and find another home for it.


/Andy



Would it be possible to update the JEP with all the CLI options? That 
would be useful for review and also useful for those invoking it with 
the ToolProvider API.

Done.


If I read the webrev correctly then it adds two modules, one with the 
jpackager tool and the other with an API. It would be useful to get a 
bit more information on the split. Also I think the name of the API 
module and the package that it exports needs discussion to make sure 
that the right names are chosen.
Yes - though we are currently using jdk.packager.services, we are open 
to other suggestions as the name for these. "jdk.packager.runtime" has 
also been suggested.


-Alan

/Andy

Re: Stop using precompiled headers for Linux?

[Aleksey Shipilev]

On 10/30/2018 06:26 PM, Ioi Lam wrote:
> Is there any advantage of using precompiled headers on Linux? 

I have measured it recently on shenandoah repositories, and fastdebug/release 
build times have not
improved with or without PCH. Actually, it gets worse when you touch a single 
header that is in PCH
list, and you end up recompiling the entire Hotspot. I would be in favor of 
disabling it by default.

> It's on by default and we keep having
> breakage where someone would forget to add #include. The latest instance is 
> JDK-8213148.

Yes, we catch most of these breakages in CIs. Which tells me adding it to 
jdk-submit would cover
most of the breakage during pre-integration testing.

-Aleksey

Re: Stop using precompiled headers for Linux?

[Erik Joelsson]

Last I checked, it did provide significant build speed improvements when 
building just hotspot, but that could need revisiting.


We do have verification of --disable-precompiled-headers (in slowdebug) 
in builds-tier2 so we normally get notified if this fails. However, 
Mach5 has not been running since Friday so this particular bug wasn't 
detected automatically. Looking at the bug, it also failed on Solaris, 
which would have been caught by tier1 builds.


/Erik


On 2018-10-30 10:26, Ioi Lam wrote:
Is there any advantage of using precompiled headers on Linux? It's on 
by default and we keep having breakage where someone would forget to 
add #include. The latest instance is JDK-8213148.


I just turn on precompiled headers explicitly in all my builds. I 
don't see any difference in build time (at least not significant 
enough for me to bother).


Should we disable it by default on Linux?

Thanks

- Ioi

Re: Stop using precompiled headers for Linux?

[Thomas Stüfe]

It would help already if Oracle would disable precompiled headers for
the submit test builds.

..Thomas
On Tue, Oct 30, 2018 at 6:26 PM Ioi Lam  wrote:
>
> Is there any advantage of using precompiled headers on Linux? It's on by
> default and we keep having breakage where someone would forget to add
> #include. The latest instance is JDK-8213148.
>
> I just turn on precompiled headers explicitly in all my builds. I don't
> see any difference in build time (at least not significant enough for me
> to bother).
>
> Should we disable it by default on Linux?
>
> Thanks
>
> - Ioi
>
>

Re: Stop using precompiled headers for Linux?

[Roman Kennke]

I'd be in favour of disabling by default.

Roman

> Is there any advantage of using precompiled headers on Linux? It's on by
> default and we keep having breakage where someone would forget to add
> #include. The latest instance is JDK-8213148.
> 
> I just turn on precompiled headers explicitly in all my builds. I don't
> see any difference in build time (at least not significant enough for me
> to bother).
> 
> Should we disable it by default on Linux?
> 
> Thanks
> 
> - Ioi
> 
> 

Stop using precompiled headers for Linux?

[Ioi Lam]

Is there any advantage of using precompiled headers on Linux? It's on by 
default and we keep having breakage where someone would forget to add 
#include. The latest instance is JDK-8213148.


I just turn on precompiled headers explicitly in all my builds. I don't 
see any difference in build time (at least not significant enough for me 
to bother).


Should we disable it by default on Linux?

Thanks

- Ioi

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

[Kevin Rushforth]

inline

On 10/30/2018 9:09 AM, Alan Bateman wrote:

On 30/10/2018 14:11, Andy Herrick wrote:


:


If I read the webrev correctly then it adds two modules, one with 
the jpackager tool and the other with an API. It would be useful to 
get a bit more information on the split. Also I think the name of 
the API module and the package that it exports needs discussion to 
make sure that the right names are chosen.
Yes - though we are currently using jdk.packager.services, we are 
open to other suggestions as the name for these. 
"jdk.packager.runtime" has also been suggested.
Alex has suggested jdk.jpackager to avoid giving the impression that 
it's the "JDK packager". Also several existing tool modules have the 
tool name in the module name (jdk.jdeps, jdk.jlink, jdk.jshell, ...).


Yes, I think jdk.jpackager is a fine name for this module.

Is the API to ensure that only one instance of the application is 
running really tied to the jpackager tool? Could it be used by 
applications that aren't packaged in with this tool? I'm asking in 
case there is a better name for this API module.


In its current form it is tied to jpackager. Andy might be able to 
comment on how tightly-coupled it is, and what it would take to 
generalize it, but that wasn't a goal to make it usable for apps that 
aren't packaged using jpackager. Other things that will likely go into 
this runtime support module in the future:


* Service daemon support
* User JVM args support

-- Kevin

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

[Alan Bateman]

On 30/10/2018 14:11, Andy Herrick wrote:

:


What is the status of the JNLPConverter tool? I see it is included as 
a "demo" but maybe it would be better to host somewhere else as this 
is for developers migrating Java Web Start applications.

Our current plan is to deliver it only as a demo.
This looks like a migration tool rather than a demo so not clear to me 
that this is the right approach. Also as the tool is for migration from 
Java Web Start when maybe it should be a tool hosted with the Oracle JDK 
download rather than something to put into the JDK.




:


If I read the webrev correctly then it adds two modules, one with the 
jpackager tool and the other with an API. It would be useful to get a 
bit more information on the split. Also I think the name of the API 
module and the package that it exports needs discussion to make sure 
that the right names are chosen.
Yes - though we are currently using jdk.packager.services, we are open 
to other suggestions as the name for these. "jdk.packager.runtime" has 
also been suggested.
Alex has suggested jdk.jpackager to avoid giving the impression that 
it's the "JDK packager". Also several existing tool modules have the 
tool name in the module name (jdk.jdeps, jdk.jlink, jdk.jshell, ...).


Is the API to ensure that only one instance of the application is 
running really tied to the jpackager tool? Could it be used by 
applications that aren't packaged in with this tool? I'm asking in case 
there is a better name for this API module.


-Alan

Re: RFR: JDK-8210958 Rename "make run-test" to "make test"

[Erik Joelsson]

Looks good.

/Erik


On 2018-10-30 00:41, Magnus Ihse Bursie wrote:

On 2018-10-23 10:45, Magnus Ihse Bursie wrote:


On 2018-10-19 18:11, Erik Joelsson wrote:

Looks good, and thanks for fixing test-make. I found the same issue 
today.


Could you also change the make target we call for 
run-test/run-test-prebuilt profiles in jib-profiles.js?


There's no target for run-test in jib-profiles.js; only for 
run-test-prebuilt (as far as I can understand, at least). I've 
updated this, and I also renamed all references to "run-test" to just 
"test". Maybe this was going too far. Let me know if you want me to 
revert that part.


Updated webrev (only jib-profiles.js has changed):

http://cr.openjdk.java.net/~ihse/JDK-8210958-rename-run-test-to-test/webrev.03 

By way of private discussion, I learned that this had a bit too much 
repercussions. I've reverted the change to jib-profiles.js, and only 
changed the name of the target.


New webrev:
http://cr.openjdk.java.net/~ihse/JDK-8210958-rename-run-test-to-test/webrev.04 



(only change is in jib-profiles.js)

/Magnus



/Magnus


/Erik


On 2018-10-19 05:32, Magnus Ihse Bursie wrote:

Now that JDK-8212028 is pushed, it's time to revisit this.

In the meantime, the compile-command test had been added, which 
caused me to realize some better handling of the make tests were 
needed. These fixes are also included. Now "make test-make" will 
run all build system tests with proper dependencies.


I also fixed a test in TestMakeBase.gmk which became broken with 
JDK-8212028.


Updated webrev:
http://cr.openjdk.java.net/~ihse/JDK-8210958-rename-run-test-to-test/webrev.02 



/Magnus


On 2018-09-20 12:47, Magnus Ihse Bursie wrote:
The time has come to start phasing out the old test running 
framework ("cd test && make"). This patch will change the behavior 
of "make test" to use the new run-test framework, instead of the old.


The old framework is still available as of now by using "cd test 
&& make".


The "run-test" target (and variants like exploded-run-test or 
run-test-tier1) are kept as aliases.


Using "cd test && make" will result in a warning being printed 
that it is recommended to stop using this way of running tests.


Bug: https://bugs.openjdk.java.net/browse/JDK-8210958
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8210958-rename-run-test-to-test/webrev.01


/Magnus

Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

[Andy Herrick]

On 10/24/2018 10:22 AM, Alan Bateman wrote:

On 23/10/2018 16:49, Andy Herrick wrote:
This patch implements the Java Packager Tool (jpackager) as described 
in JEP 343: Packaging Tool 



jpackager is a simple packaging tool, based on the JavaFX 
|javapackager| tool, that:


 * Supports native packaging formats to give the end user a natural
   installation experience. These formats include |msi| and |exe| on
   Windows, |pkg| and |dmg| on MacOS, and |deb| and |rpm| on Linux.
 * Allows launch-time parameters to be specified at packaging time.
 * Can be invoked directly, from the command line, or programmatically,
   via the |ToolProvider| API.

Webrev:

http://cr.openjdk.java.net/~herrick/8212780/webrev.01/


cc'ing build-dev as it's important to get it reviewed there.

What is the plan for tests to go with this tool? I see there is one 
test in the webrev to do some argument validation but I don't see 
anything else right now.
We plan to incorporate the initial feedback from this review, and 
include an initial set of automated tests in a refresh sometime next week.

We will continue to develop and automate tests for future updates.


What is the status of the JNLPConverter tool? I see it is included as 
a "demo" but maybe it would be better to host somewhere else as this 
is for developers migrating Java Web Start applications.

Our current plan is to deliver it only as a demo.


Would it be possible to update the JEP with all the CLI options? That 
would be useful for review and also useful for those invoking it with 
the ToolProvider API.

Done.


If I read the webrev correctly then it adds two modules, one with the 
jpackager tool and the other with an API. It would be useful to get a 
bit more information on the split. Also I think the name of the API 
module and the package that it exports needs discussion to make sure 
that the right names are chosen.
Yes - though we are currently using jdk.packager.services, we are open 
to other suggestions as the name for these. "jdk.packager.runtime" has 
also been suggested.


-Alan

/Andy

RFR: 8209093 - JEP 340: One AArch64 Port, Not Two

[Bob Vandette]

This JEP is now targeted to JDK 12.

I’ve merged up to the latest sources and will be integrating these changes in 
the next
day or two.

Here’s the latest webrev post merge in the event anyone spots any last minute 
issues.

http://cr.openjdk.java.net/~bobv/8209093/webrev.02 


Bob,.

Re: RFR: JDK-8210958 Rename "make run-test" to "make test"

[Magnus Ihse Bursie]

On 2018-10-23 10:45, Magnus Ihse Bursie wrote:


On 2018-10-19 18:11, Erik Joelsson wrote:

Looks good, and thanks for fixing test-make. I found the same issue 
today.


Could you also change the make target we call for 
run-test/run-test-prebuilt profiles in jib-profiles.js?


There's no target for run-test in jib-profiles.js; only for 
run-test-prebuilt (as far as I can understand, at least). I've updated 
this, and I also renamed all references to "run-test" to just "test". 
Maybe this was going too far. Let me know if you want me to revert 
that part.


Updated webrev (only jib-profiles.js has changed):

http://cr.openjdk.java.net/~ihse/JDK-8210958-rename-run-test-to-test/webrev.03 

By way of private discussion, I learned that this had a bit too much 
repercussions. I've reverted the change to jib-profiles.js, and only 
changed the name of the target.


New webrev:
http://cr.openjdk.java.net/~ihse/JDK-8210958-rename-run-test-to-test/webrev.04

(only change is in jib-profiles.js)

/Magnus



/Magnus


/Erik


On 2018-10-19 05:32, Magnus Ihse Bursie wrote:

Now that JDK-8212028 is pushed, it's time to revisit this.

In the meantime, the compile-command test had been added, which 
caused me to realize some better handling of the make tests were 
needed. These fixes are also included. Now "make test-make" will run 
all build system tests with proper dependencies.


I also fixed a test in TestMakeBase.gmk which became broken with 
JDK-8212028.


Updated webrev:
http://cr.openjdk.java.net/~ihse/JDK-8210958-rename-run-test-to-test/webrev.02 



/Magnus


On 2018-09-20 12:47, Magnus Ihse Bursie wrote:
The time has come to start phasing out the old test running 
framework ("cd test && make"). This patch will change the behavior 
of "make test" to use the new run-test framework, instead of the old.


The old framework is still available as of now by using "cd test && 
make".


The "run-test" target (and variants like exploded-run-test or 
run-test-tier1) are kept as aliases.


Using "cd test && make" will result in a warning being printed that 
it is recommended to stop using this way of running tests.


Bug: https://bugs.openjdk.java.net/browse/JDK-8210958
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8210958-rename-run-test-to-test/webrev.01


/Magnus