Re: RFR: JDK-8036003: Add --with-debug-symbols=[none|internal|external|zipped]

[Magnus Ihse Bursie]

On 2015-12-11 00:47, Yasumasa Suenaga wrote:

Hi build folks,

Could you review it?


http://cr.openjdk.java.net/~ysuenaga/JDK-8036003/webrev.06/


I like the new way of passing a help text for deprecation. Just a note, 
in the help text you called the new option "--with-debug-symbols" 
instead of "--with-native-debug-symbols", which is kind of unhelpful. 
;-) You can just fix this, no need to upload a new webrev.


So, if you just fix this, I'm okay with the patch.

Thank you for fighting this all the way through. I do apologize for the 
slow process.


/Magnus

Re: RFR: 8073139 PPC64: User-visible arch directory and os.arch value on ppc64le cause issues with Java tooling

[Volker Simonis]

Hi Sasha, David,

I've just verified that your patch still works for linux/ppc64. I've
built and smoke-tested both linux/ppc64 and ppc64le and everything
looks good. So from my side this is ready to push.

Thanks for finally getting this ready!
Volker


On Fri, Dec 11, 2015 at 5:47 AM, David Holmes  wrote:
> On 11/12/2015 2:15 PM, Alexander Smundak wrote:
>>
>> I have jcheck enabled, so I am surprised that the absence of the
>> Reviewed-by: field was not flagged. Anyways, here's the new set:
>
>
> Not sure what is going on with your jcheck setup but the formatting is still
> wrong:
>
> - the changeset "user" must be OpenJDK username not email address
> - reviewers must be OpenJDK user names not email addresses
> - contributed-by attribution should be simple email address or the following
> is also allowed:
>   Andrew Hughes 
>
> Sorry.
>
> David
> -
>
>
>> http://cr.openjdk.java.net/~asmundak/8073139/hotspot/webrev.04
>> http://cr.openjdk.java.net/~asmundak/8073139/jdk/webrev.04
>> http://cr.openjdk.java.net/~asmundak/8073139/root/webrev.04
>>
>> Note that I don't have access to the big-endian PowerPC64. The
>> original patch was verified by goetz.lindenma...@sap.com, but it has
>> changed since. Goetz, can you verify that the new patch still works?
>>
>> Sasha
>>
>> On Wed, Dec 9, 2015 at 8:59 PM, David Holmes 
>> wrote:
>>>
>>> On 10/12/2015 4:16 AM, Alexander Smundak wrote:


 I am confused -- is there anything you want me to change in the existing
 set:

 http://cr.openjdk.java.net/~asmundak/8073139/hotspot/webrev.03
 http://cr.openjdk.java.net/~asmundak/8073139/jdk/webrev.03
 http://cr.openjdk.java.net/~asmundak/8073139/root/webrev.03
>>>
>>>
>>>
>>> Yes they need to be created using the correct, jcheck[1] compliant,
>>> commit
>>> messages:
>>>
>>> http://openjdk.java.net/guide/producingChangeset.html
>>>
>>> Mainly Reviewers seem to be missing - but I'm also not sure other details
>>> are jcheck compliant.
>>>
>>> Thanks,
>>> David
>>>
>>> [1] http://openjdk.java.net/projects/code-tools/jcheck/
>>>
>>>
 Sasha

 On Tue, Dec 8, 2015 at 7:10 PM, David Holmes 
 wrote:
>
>
> I can sponsor for you Sasha. Just email me the changeset, or a link
> thereto.
>
> Thanks,
> David
>
>
> On 9/12/2015 3:44 AM, Alexander Smundak wrote:
>>
>>
>>
>> Thank you for the review.
>> If everyone involved is satisfied with the patch, I need a sponsor.
>>
>> Sasha
>>
>> On Mon, Dec 7, 2015 at 8:46 PM, David Holmes 
>> wrote:
>>>
>>>
>>>
>>> On 8/12/2015 1:44 PM, Alexander Smundak wrote:




 It is achievable by adding a small ppc-specific check to the
 common/autoconf/platform.m4:

 @@ -282,6 +282,8 @@
elif test "x$OPENJDK_TARGET_OS" != xmacosx && test
 "x$OPENJDK_TARGET_CPU" = xx86_64; then
  # On all platforms except MacOSX replace x86_64 with amd64.
  OPENJDK_TARGET_CPU_LEGACY="amd64"
 +  elif test "x$OPENJDK_TARGET_CPU" = xppc64le; then
 +OPENJDK_TARGET_CPU_LEGACY="ppc64"
fi
AC_SUBST(OPENJDK_TARGET_CPU_LEGACY)

 However, there is a code in make/Images.gmk using
 OPENJDK_TARGET_CPU_LEGACY
   $(call info-file-item, "OS_ARCH",
 "$(OPENJDK_TARGET_CPU_LEGACY)")
 so that the 'release' file in the image directory will now have
 JAVA_VERSION="9"
 JAVA_FULL_VERSION="9-internal+0-2015-12-07-190811.asmundak.hs-rt"
 OS_NAME="Linux"
 OS_VERSION="2.6"
 OS_ARCH="ppc64"
 
 SOURCE=..,

 instead of "ppc64le".

 If someone can tell me which other variable I should use to achieve
 that without changing the contents of the 'release' file on other
 platforms, I'll be grateful.
>>>
>>>
>>>
>>>
>>>
>>> Okay never mind - and thanks for looking into this. I see now this is
>>> set
>>> via:
>>>
>>> ./autoconf/flags.m4:  COMMON_CCXXFLAGS_JDK="$COMMON_CCXXFLAGS_JDK
>>> -DARCH='\"$OPENJDK_TARGET_CPU_LEGACY\"' -D$OPENJDK_TARGET_CPU_LEGACY"
>>>
>>> so there is no way to add a second value without introducing some
>>> additional
>>> variable. At the hotspot level it could be handled differently if we
>>> had
>>> a
>>> platform_ppc64le file, as it could just be added to the SYSDEFS.
>>>
>>> Thanks,
>>> David

Re: RFR: JDK-8144312: Remove limitations on the default number of jobs in the build

[David Holmes]

On 11/12/2015 11:16 PM, Magnus Ihse Bursie wrote:

On 2015-12-03 03:11, Roger Riggs wrote:

Hi,

It would be useful to figure out the number of cpus available when in
a container.
Some comments have added to:
8140793 
getAvailableProcessors may incorrectly report the number of cpus in
Docker container

But so far we haven't dug deep enough.   Suggestions are welcome?

http://serverfault.com/questions/691659/count-number-of-allowed-cpus-in-a-docker-container
suggests running nproc. I'm not sure if that can be counted on to be
present, but we could certainly check for it.


I'd like to know how nproc does it so we can try to apply the same logic 
in the VM for Runtime.availableProcessors. Can someone actually confirm 
that it returns the number of processors available to the container?


David


/Magnus



Roger


On 12/2/15 6:59 PM, Martin Buchholz wrote:

Not to say you shouldn't do this, but I worry that increasingly
computing
is being done in "containers" where e.g. the number of cpus is doubling
every year but only a small number are available to actually be used
by a
given process.  if availableProcessors reports 1 million, what should we
do?  (no need to answer...)

On Tue, Dec 1, 2015 at 1:55 AM, Erik Joelsson 
wrote:


Hello,

The current heuristic for figuring out what to default set the -j
flag to
make needs some tweaking.

In JDK 9, it looks at the amount of memory and the number of cpus in
the
system. It divides memory by 1024 to get a safe number of jobs that
will
fit into memory. The lower of that number and the number of cpus is
then
picked. The number is then scaled down to about 90% of the number of
cpus
to leave some resources for other activities. It is also capped at 16.

Since we now have the build using "nice" to make sure the build isn't
bogging down the system, I see no reason to do the 90% scaling anymore.
Also, the performance issues that forced us to cap at 16 have long been
fixed, and even if we don't scale well beyond 16, we do still scale.
So I
propose we remove that arbitrary limitation too.

Bug: https://bugs.openjdk.java.net/browse/JDK-8144312
Webrev: http://cr.openjdk.java.net/~erikj/8144312/webrev.01/

/Erik

Re: RFR: JDK-8144312: Remove limitations on the default number of jobs in the build

[Magnus Ihse Bursie]

On 2015-12-03 03:11, Roger Riggs wrote:

Hi,

It would be useful to figure out the number of cpus available when in 
a container.

Some comments have added to:
8140793  
getAvailableProcessors may incorrectly report the number of cpus in 
Docker container


But so far we haven't dug deep enough.   Suggestions are welcome?
http://serverfault.com/questions/691659/count-number-of-allowed-cpus-in-a-docker-container 
suggests running nproc. I'm not sure if that can be counted on to be 
present, but we could certainly check for it.


/Magnus



Roger


On 12/2/15 6:59 PM, Martin Buchholz wrote:
Not to say you shouldn't do this, but I worry that increasingly 
computing

is being done in "containers" where e.g. the number of cpus is doubling
every year but only a small number are available to actually be used 
by a

given process.  if availableProcessors reports 1 million, what should we
do?  (no need to answer...)

On Tue, Dec 1, 2015 at 1:55 AM, Erik Joelsson 
wrote:


Hello,

The current heuristic for figuring out what to default set the -j 
flag to

make needs some tweaking.

In JDK 9, it looks at the amount of memory and the number of cpus in 
the
system. It divides memory by 1024 to get a safe number of jobs that 
will
fit into memory. The lower of that number and the number of cpus is 
then
picked. The number is then scaled down to about 90% of the number of 
cpus

to leave some resources for other activities. It is also capped at 16.

Since we now have the build using "nice" to make sure the build isn't
bogging down the system, I see no reason to do the 90% scaling anymore.
Also, the performance issues that forced us to cap at 16 have long been
fixed, and even if we don't scale well beyond 16, we do still scale. 
So I

propose we remove that arbitrary limitation too.

Bug: https://bugs.openjdk.java.net/browse/JDK-8144312
Webrev: http://cr.openjdk.java.net/~erikj/8144312/webrev.01/

/Erik

Re: RFR: JDK-8145115: make JAVAC_FLAGS=-g no longer works

[Severin Gehwolf]

On Fri, 2015-12-11 at 14:10 +0100, Magnus Ihse Bursie wrote:
> On 2015-12-10 16:02, Severin Gehwolf wrote:
> > Yes, pretty please :) This is a long standing issue (not just java
> > class debug info, but debug info in general). See
> > JDK-8036003 with review thread:
> > http://mail.openjdk.java.net/pipermail/build-dev/2015-December/016155.html
> > 
> > Once JDK-8036003 is fixed we'd need something similar for Java classes
> > and we can get rid of invoking make with build internal variables.
> 
> I agree. Yasumasa Suenaga had a pretty good implementation in 
> http://cr.openjdk.java.net/~ysuenaga/JDK-8036003/webrev.04/.
> 
> I think it could form the basis of getting a proper java debug symbol 
> support flag to configure. My main issue with that is that it must also 
> work with the case  "--enable-debug --disable-java-debug-symbols", to 
> produce a build without java debug symbols. This means that this logic 
> need to interact with the logic in flags.m4:
>    case $DEBUG_LEVEL in
>  fastdebug | slowdebug )
>    CFLAGS_JDK="$CFLAGS_JDK $CFLAGS_DEBUG_SYMBOLS $CFLAGS_DEBUG_OPTIONS"
>    CXXFLAGS_JDK="$CXXFLAGS_JDK $CXXFLAGS_DEBUG_SYMBOLS 
> $CXXFLAGS_DEBUG_OPTIONS"
>    JAVAC_FLAGS="$JAVAC_FLAGS -g"
> 
> Without digging a bit deeper in the source, I can't say the best way to 
> achieve this. Perhaps the configure flag check should set a variable 
> like JAVA_DEBUG_SYMBOLS:=true/false/default that can be checked in 
> flags.m4? Or we should defer the modification of JAVAC_FLAGS.

OK. Thanks for the info. I've created this bug for this enhancement:
https://bugs.openjdk.java.net/browse/JDK-8145188

Cheers,
Severin

Re: RFR: JDK-8145115: make JAVAC_FLAGS=-g no longer works

[Magnus Ihse Bursie]

On 2015-12-10 16:02, Severin Gehwolf wrote:

Yes, pretty please :) This is a long standing issue (not just java
class debug info, but debug info in general). See
JDK-8036003 with review thread:
http://mail.openjdk.java.net/pipermail/build-dev/2015-December/016155.html

Once JDK-8036003 is fixed we'd need something similar for Java classes
and we can get rid of invoking make with build internal variables.


I agree. Yasumasa Suenaga had a pretty good implementation in 
http://cr.openjdk.java.net/~ysuenaga/JDK-8036003/webrev.04/.


I think it could form the basis of getting a proper java debug symbol 
support flag to configure. My main issue with that is that it must also 
work with the case  "--enable-debug --disable-java-debug-symbols", to 
produce a build without java debug symbols. This means that this logic 
need to interact with the logic in flags.m4:

  case $DEBUG_LEVEL in
fastdebug | slowdebug )
  CFLAGS_JDK="$CFLAGS_JDK $CFLAGS_DEBUG_SYMBOLS $CFLAGS_DEBUG_OPTIONS"
  CXXFLAGS_JDK="$CXXFLAGS_JDK $CXXFLAGS_DEBUG_SYMBOLS 
$CXXFLAGS_DEBUG_OPTIONS"

  JAVAC_FLAGS="$JAVAC_FLAGS -g"

Without digging a bit deeper in the source, I can't say the best way to 
achieve this. Perhaps the configure flag check should set a variable 
like JAVA_DEBUG_SYMBOLS:=true/false/default that can be checked in 
flags.m4? Or we should defer the modification of JAVAC_FLAGS.


/Magnus

Re: RFR: JDK-8145115: make JAVAC_FLAGS=-g no longer works

[Magnus Ihse Bursie]

On 2015-12-10 15:49, Erik Joelsson wrote:
I looked closer at your actual command lines and managed to reproduce. 
The problem is JAVAC_FLAGS=-g. We have unfortunately used that name 
for a local variable in make/CompileJavaModules.gmk. Fix is pretty 
simple, just rename that local variable. In the longer term, we need 
to be able to enable -g for javac with a configure argument instead.


Bug: https://bugs.openjdk.java.net/browse/JDK-8145115
Patch:
diff -r a151b3ec17a1 make/CompileJavaModules.gmk
--- a/make/CompileJavaModules.gmk
+++ b/make/CompileJavaModules.gmk
@@ -556,7 +556,7 @@
   ifneq ($(BUILD_CRYPTO), true)
   CLASSPATH += $(JDK_OUTPUTDIR)/modules/$(MODULE)
   endif
-JAVAC_FLAGS := -bootclasspath $(EMPTY_DIR) -extdirs $(EMPTY_DIR) \
+JAVAC_FLAGS_BOOTCLASSPATH := -bootclasspath $(EMPTY_DIR) -extdirs 
$(EMPTY_DIR) \

 -endorseddirs $(EMPTY_DIR) $($(MODULE)_ADD_JAVAC_FLAGS)

 $(eval $(call SetupJavaCompilation, $(MODULE), \
@@ -566,7 +566,7 @@
 BIN := $(if $($(MODULE)_BIN), $($(MODULE)_BIN), 
$(JDK_OUTPUTDIR)/modules/$(MODULE)), \

 HEADERS := $(SUPPORT_OUTPUTDIR)/headers/$(MODULE), \
 CLASSPATH := $(CLASSPATH), \
-ADD_JAVAC_FLAGS := $($(MODULE)_ADD_JAVAC_FLAGS) $(JAVAC_FLAGS) \
+ADD_JAVAC_FLAGS := $($(MODULE)_ADD_JAVAC_FLAGS) 
$(JAVAC_FLAGS_BOOTCLASSPATH) \

 ))
 TARGETS += $($(MODULE)) $($(MODULE)_COPY_EXTRA)



Patch looks good to me.

/Magnus


/Erik

On 2015-12-09 18:53, Severin Gehwolf wrote:

Hi Erik,

On Wed, 2015-12-09 at 17:35 +0100, Erik Joelsson wrote:

I'm not able to reproduce with either OracleJDK 8 or 8u66.

The simplified reproducer is (extracted from build.log):
( (/usr/bin/bash 
/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk-9-hs-rt-testclone/common/bin/logger.sh 
/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk-9-hs-rt-testclone/build/linux-x86_64-normal-server-release/jdk/modules/java.logging/_the.java.logging_batch.log 
/usr/lib/jvm/java-1.8.0-openjdk/bin/java -XX:+UseSerialGC -Xms32M 
-Xmx512M 
"-Xbootclasspath/p:/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk-9-hs-rt-testclone/build/linux-x86_64-normal-server-release/buildtools/interim_langtools.jar" 
-cp 
/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk-9-hs-rt-testclone/build/linux-x86_64-normal-server-release/buildtools/interim_langtools.jar 
com.sun.tools.sjavac.Main 
--server:portfile=/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk-9-hs-rt-testclone/build/linux-x86_64-normal-server-release/make-support/javacservers/server.port,id=java.logging,sjavac=/usr/lib/jvm/java-1.8.0-openjdk/bin/java%20-d64%20-Xms512M%20-Xmx2048M%20"-Xbootclasspath/p:/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk-9-hs-rt-testclone/build/linux-x86_64-normal-server-release/buildtools/interim_langtools.jar"%20-cp%20/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk-9-hs-rt-testclone/build/linux-x86_64-normal-server-release/buildtools/interim_langtools.jar%20com.sun.tools.sjavac.Main 
-source 9 -target 9 -encoding ascii -XDignore.symbol.file=true 
-Xlint:all -Werror -g -Xdoclint:all/protected,-reference 
'-Xdoclint/package:java.*,javax.*' -g -cp 
"/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk-9-hs-rt-testclone/build/linux-x86_64-normal-server-release/jdk/modules/java.base:/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk-9-hs-rt-testclone/build/linux-x86_64-normal-server-release/jdk/modules/java.logging" 
-implicit:none -d 
/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk-9-hs-rt-testclone/build/linux-x86_64-normal-server-release/jdk/modules/java.logging 
-h 
/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk-9-hs-rt-testclone/build/linux-x86_64-normal-server-release/support/headers/java.logging.java.logging.tmp 
@/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk-9-hs-rt-testclone/build/linux-x86_64-normal-server-release/jdk/modules/java.logging/_the.java.logging_batch.tmp 
&& /usr/bin/rm -f 
/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk-9-hs-rt-testclone/build/linux-x86_64-normal-server-release/jdk/modules/java.logging/_the.java.logging_batch.log) 
|| (exitcode=$? && /usr/bin/mv 
/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk-9-hs-rt-testclone/build/linux-x86_64-normal-server-release/jdk/modules/java.logging/_the.java.logging_batch.log 
/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk-9-hs-rt-testclone/build/linux-x86_64-normal-server-release/make-support/failure-logs/java.logging.log 
&& exit $exitcode) ) && /usr/bin/mv 
/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk-9-hs-rt-testclone/build/linux-x86_64-normal-server-release/jdk/modules/java.logging/_the.java.logging_batch.tmp 
/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk-9-hs-rt-testclone/build/linux-x86_64-normal-server-release/jdk/modules/java.logging/_the.java.logging_batch 



I'm also attaching the failing command invocation as a file (Putting
this into a file and re-executing it reproduces the problem for 

Re: RFR: JDK-8036003: Add --with-debug-symbols=[none|internal|external|zipped]

[Yasumasa Suenaga]

Hi Magnus,

Thank you for reviewing!


I like the new way of passing a help text for deprecation. Just a note, in the help text you called 
the new option "--with-debug-symbols" instead of "--with-native-debug-symbols", 
which is kind of unhelpful. ;-) You can just fix this, no need to upload a new webrev.


Sorry, I've fixed it and pushed.


Thank you so much!

Yasumasa


On 2015/12/11 20:52, Magnus Ihse Bursie wrote:

On 2015-12-11 00:47, Yasumasa Suenaga wrote:

Hi build folks,

Could you review it?


http://cr.openjdk.java.net/~ysuenaga/JDK-8036003/webrev.06/


I like the new way of passing a help text for deprecation. Just a note, in the help text you called 
the new option "--with-debug-symbols" instead of "--with-native-debug-symbols", 
which is kind of unhelpful. ;-) You can just fix this, no need to upload a new webrev.

So, if you just fix this, I'm okay with the patch.

Thank you for fighting this all the way through. I do apologize for the slow 
process.

/Magnus

[urgent] RFR: JDK-8145206: Configure broken on Macosx

[Erik Joelsson]

JDK-8036003 broke configure on Macosx:

checking what type of native debug symbols to use... zipped
configure: error: Unable to find objcopy, cannot enable native debug 
symbols


We can only fail for missing objcopy on platforms that actually use it.

Bug: https://bugs.openjdk.java.net/browse/JDK-8145206
Webrev: http://cr.openjdk.java.net/~erikj/8145206/webrev.01/

/Erik

Re: RFR: JDK-8145185: Rename JAB the build tool to JIB

[Magnus Ihse Bursie]

Looks good to me. 

/Magnus

> 11 dec. 2015 kl. 15:50 skrev Erik Joelsson :
> 
> Hello,
> 
> Since the acronym JAB was already taken by Java Access Bridge, we nee dto 
> find a new name for the new build tool. The new name will be JIB - The JDK 
> Integrated Build tool. All references and file names associated with JAB in 
> the JDK 9 repos need to be changed to JIB.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8145185
> Webrev: http://cr.openjdk.java.net/~erikj/8145185/webrev.01/
> 
> /Erik

RFR: 8145132: Initial updates for ios x86_64 mobile/dev builds.

[Gary Adams]

Here's the initial upload of changes that provides support for the ios 
and android ports
for the mobile/dev repos. While there have been some preliminary reviews 
of the code,
there is still more work required before we will look for more thorough 
reviews

and an integration to mobile/jdk9 repos.

  Issue: https://bugs.openjdk.java.net/browse/JDK-8145132
  Webrev: http://cr.openjdk.java.net/~gadams/8145132/webrev.00/


Here's a simple configure script to generate a ios-x86_64 build for use
with the iphone simulator. (uses homebrew 64 bit freetype from pkgconfig)

export JAVA_HOME=`/usr/libexec/java_home -v 1.8`
export PATH=$JAVA_HOME/bin:~/homebrew/bin:$PATH

bash ../../configure \
--openjdk-target=x86_64-macos-ios \
--with-boot-jdk=$JAVA_HOME \
--disable-warnings-as-errors \
--disable-headful \
--enable-static-build=yes \
--with-jvm-variants=minimal1


Also, tested with i586-macos-ios target for 32 bit
with a locally built --with-freetype 2.6.2.

Re: [urgent] RFR: JDK-8145206: Configure broken on Macosx

[Tim Bell]

Erik:


JDK-8036003 broke configure on Macosx:

checking what type of native debug symbols to use... zipped
configure: error: Unable to find objcopy, cannot enable native debug 
symbols


We can only fail for missing objcopy on platforms that actually use it.

Bug: https://bugs.openjdk.java.net/browse/JDK-8145206
Webrev: http://cr.openjdk.java.net/~erikj/8145206/webrev.01/


Looks good to me.

Tim

Re: RFR: JDK-8145185: Rename JAB the build tool to JIB

[Tim Bell]

Hi Erik

Looks good to me as well.

/Tim

On 12/11/15 07:43, Magnus Ihse Bursie wrote:

Looks good to me.

/Magnus


11 dec. 2015 kl. 15:50 skrev Erik Joelsson :

Hello,

Since the acronym JAB was already taken by Java Access Bridge, we nee dto find 
a new name for the new build tool. The new name will be JIB - The JDK 
Integrated Build tool. All references and file names associated with JAB in the 
JDK 9 repos need to be changed to JIB.

Bug: https://bugs.openjdk.java.net/browse/JDK-8145185
Webrev: http://cr.openjdk.java.net/~erikj/8145185/webrev.01/

/Erik

Re: RFR: JDK-8142907 Integration of minor fixes from the build-infra project

[Magnus Ihse Bursie]

Hi David,

Resurrecting the second part of the build-infra integrations. :) 
Fortunately, no major code changes in the meantime has affected this 
patch. (I did need to update the new 
hotspot/make/lib/Lib-jdk.hotspot.agent.gmk though.)



On 2015-11-16 05:33, David Holmes wrote:

Hi Magnus,

I had a flick through most of the files. Overall seems okay but I have 
a few queries below.


Replies inline.



On 13/11/2015 12:43 PM, Magnus Ihse Bursie wrote:

The build-infra project has collected a number of minor fixes and
changes during the new hotspot build development. Its a mix of code
cleanup and new capabilities.

Not all of these new features are immediately beneficial to the JDK, but
they will be needed for the upcoming new Hotspot build, and it will not
hurt to have them in mainline. (In fact, it will tremendously help
merging between mainline and build-infra.)

The fix addresses these issues:

In general:
* Break out hotspot configuration into hotspot.m4
* Long link lines uses @-files
* Consistently use -Wl instead of -Xlinker
* Improve clang on linux compilation
* Set shared library name explicitely on solaris
* Set correct shared library flag on Windows (-dll)
* Consistency fixes for build toolchain
* Bring compare script up to date
* General code/whitespace cleanup
* Additional functionality in MakeBase

In NativeCompilation.gmk:
* More efficient vardeps for per-file CFLAGS
* Fewer shell executions (means better performance on Windows)
* EXCLUDE_PATTERN and EXTRA_OBJECT_FILES
* Debug symbols on macosx (disabled for existing code to keep current
behavior)

Enabling debug info on macosx on existing jdk should be treated in a
follow-up bug.
Bug: https://bugs.openjdk.java.net/browse/JDK-8142907
WebRev:
http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.01 




(It turned out that WebRev could not at the same time include files from
multiple repos and track the history of a "hg cp":ied file. So I created
an alternative revision here:
http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.02/ 




It does not include the jdk files, but hotspot.m4 might be easier to
understand)


flags.m4:

  60   AC_SUBST(LEGACY_EXTRA_CFLAGS)
  61   AC_SUBST(LEGACY_EXTRA_CXXFLAGS)
  62   AC_SUBST(LEGACY_EXTRA_LDFLAGS)
  63
  64   AC_SUBST(EXTRA_CFLAGS)
  65   AC_SUBST(EXTRA_CXXFLAGS)
  66   AC_SUBST(EXTRA_LDFLAGS)

IIRC we added the legacy flags purely to pass the cross-compilation 
args through to hotspot. Not sure why we need both legacy and 
non-legancy variants now ??


This is part of an ongoing effort to split CFLAGS_JDK into more 
reasonable parts. The current model is that we throw all possible flags 
together in CFLAGS_JDK (and then further on to CFLAGS_JDKLIB et al). 
While it was questionable even before, it did work when we only built 
the JDK native libraries. Now that we need to build libjvm.so as well, 
which has (for historical reason) a set of flags that are both partially 
the same and partially different, this is not so good.


We would like to see a change to a situation where the different "parts" 
that we build CFLAGS_JDK from would be handled separately, and then 
combined as needed for JDK libraries, and for Hotspot. So, currently we 
use EXTRA_CFLAGS just to add them to CFLAGS_JDK, and then drop them. In 
the new Hotspot build, we use the EXTRA_CFLAGS to add them to the 
Hotspot flags. Of course, we could have used LEGACY_EXTRA_CFLAGS, but 
we'd like to avoid using "LEGACY" for new stuff.


With that being said, I now realize that maybe this change will not be 
needed anyway, at least not right now. Since doing a proper flag cleanup 
is a major task, we decided to copy the CFLAGS_JDK behavior for Hotspot 
flags as well in the current implementation of the new hotspot build, 
and saving the cleanup for another day (so it will not block the new 
build system). So I'll revert this part of the change for now.


On Windows -LD is a superset of -dll, so it isn't obvious the change 
is correct.

I hope Erik's reply to that was satisfactory.


---

jdk/make/lib/LibCommon.gmk

+ # Disable it here for the jdk binaries until we decide to enable them.

s/binaries/libraries/ ?

Sure, I'll fix.

Actually both this fragment and the one in 
jdk/make/launcher/LauncherCommon.gmk I find confusing - what is the 
relation with hotspot here, and the role of SetupNativeCompilation?
I hope Erik's reply to that is acceptable as well. As for the other 
changes in the jdk repo, most of them is to either standardize on -Wl, 
or to fix some places were we didn't do a proper $$ (instead of $) in an 
macro that was supposed to be eval'd. (This was incorrect before as 
well, but it broke the build when the LDFLAGS started having commas in 
them.)


/Magnus



Thanks,
David



/Magnus

Re: RFR: JDK-8142907 Integration of minor fixes from the build-infra project

[Magnus Ihse Bursie]

On 2015-12-11 22:43, Magnus Ihse Bursie wrote:

Hi David,

Resurrecting the second part of the build-infra integrations. :) 
Fortunately, no major code changes in the meantime has affected this 
patch. (I did need to update the new 
hotspot/make/lib/Lib-jdk.hotspot.agent.gmk though.)


It seems I forgot to include the link to the new webrev:

http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.05

/Magnus




On 2015-11-16 05:33, David Holmes wrote:

Hi Magnus,

I had a flick through most of the files. Overall seems okay but I 
have a few queries below.


Replies inline.



On 13/11/2015 12:43 PM, Magnus Ihse Bursie wrote:

The build-infra project has collected a number of minor fixes and
changes during the new hotspot build development. Its a mix of 
code

cleanup and new capabilities.

Not all of these new features are immediately beneficial to the JDK, 
but

they will be needed for the upcoming new Hotspot build, and it will not
hurt to have them in mainline. (In fact, it will tremendously help
merging between mainline and build-infra.)

The fix addresses these issues:

In general:
* Break out hotspot configuration into hotspot.m4
* Long link lines uses @-files
* Consistently use -Wl instead of -Xlinker
* Improve clang on linux compilation
* Set shared library name explicitely on solaris
* Set correct shared library flag on Windows (-dll)
* Consistency fixes for build toolchain
* Bring compare script up to date
* General code/whitespace cleanup
* Additional functionality in MakeBase

In NativeCompilation.gmk:
* More efficient vardeps for per-file CFLAGS
* Fewer shell executions (means better performance on Windows)
* EXCLUDE_PATTERN and EXTRA_OBJECT_FILES
* Debug symbols on macosx (disabled for existing code to keep current
behavior)

Enabling debug info on macosx on existing jdk should be treated in a
follow-up bug.
Bug: https://bugs.openjdk.java.net/browse/JDK-8142907
WebRev:
http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.01 




(It turned out that WebRev could not at the same time include files 
from
multiple repos and track the history of a "hg cp":ied file. So I 
created

an alternative revision here:
http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.02/ 




It does not include the jdk files, but hotspot.m4 might be easier to
understand)


flags.m4:

  60   AC_SUBST(LEGACY_EXTRA_CFLAGS)
  61   AC_SUBST(LEGACY_EXTRA_CXXFLAGS)
  62   AC_SUBST(LEGACY_EXTRA_LDFLAGS)
  63
  64   AC_SUBST(EXTRA_CFLAGS)
  65   AC_SUBST(EXTRA_CXXFLAGS)
  66   AC_SUBST(EXTRA_LDFLAGS)

IIRC we added the legacy flags purely to pass the cross-compilation 
args through to hotspot. Not sure why we need both legacy and 
non-legancy variants now ??


This is part of an ongoing effort to split CFLAGS_JDK into more 
reasonable parts. The current model is that we throw all possible 
flags together in CFLAGS_JDK (and then further on to CFLAGS_JDKLIB et 
al). While it was questionable even before, it did work when we only 
built the JDK native libraries. Now that we need to build libjvm.so as 
well, which has (for historical reason) a set of flags that are both 
partially the same and partially different, this is not so good.


We would like to see a change to a situation where the different 
"parts" that we build CFLAGS_JDK from would be handled separately, and 
then combined as needed for JDK libraries, and for Hotspot. So, 
currently we use EXTRA_CFLAGS just to add them to CFLAGS_JDK, and then 
drop them. In the new Hotspot build, we use the EXTRA_CFLAGS to add 
them to the Hotspot flags. Of course, we could have used 
LEGACY_EXTRA_CFLAGS, but we'd like to avoid using "LEGACY" for new stuff.


With that being said, I now realize that maybe this change will not be 
needed anyway, at least not right now. Since doing a proper flag 
cleanup is a major task, we decided to copy the CFLAGS_JDK behavior 
for Hotspot flags as well in the current implementation of the new 
hotspot build, and saving the cleanup for another day (so it will not 
block the new build system). So I'll revert this part of the change 
for now.


On Windows -LD is a superset of -dll, so it isn't obvious the change 
is correct.

I hope Erik's reply to that was satisfactory.


---

jdk/make/lib/LibCommon.gmk

+ # Disable it here for the jdk binaries until we decide to enable them.

s/binaries/libraries/ ?

Sure, I'll fix.

Actually both this fragment and the one in 
jdk/make/launcher/LauncherCommon.gmk I find confusing - what is the 
relation with hotspot here, and the role of SetupNativeCompilation?
I hope Erik's reply to that is acceptable as well. As for the other 
changes in the jdk repo, most of them is to either standardize on -Wl, 
or to fix some places were we didn't do a proper $$ (instead of $) in 
an macro that was supposed to be eval'd. (This was incorrect before as 
well, but it broke the build when the LDFLAGS started having commas in 
them.)


/Magnus

Re: RFR: JDK-8145008: Add libelf package to Linux devkit

[Magnus Ihse Bursie]

On 2015-12-11 16:00, Erik Joelsson wrote:
The devkit used for building OracleJDK on linux needs to add libelf. 
Here is the patch to the devkit creation makefiles to make that happen.


Bug: https://bugs.openjdk.java.net/browse/JDK-8145008
Patch:
diff --git a/make/devkit/Tools.gmk b/make/devkit/Tools.gmk
--- a/make/devkit/Tools.gmk
+++ b/make/devkit/Tools.gmk
@@ -82,7 +82,9 @@
 libXi libXi-devel \
 libXdmcp libXdmcp-devel \
 libXau libXau-devel \
-libgcc
+libgcc \
+elfutils elfutils-devel \
+elfutils-libelf elfutils-libelf-devel


 ifeq ($(ARCH),x86_64)


/Erik


Looks good to me.

/Magnus

Re: RFR (S): JDK-8142909 Integration of minor fixes from the build-infra project

[Magnus Ihse Bursie]

I'm trying to resurrect this review.

Here is a new version:
http://cr.openjdk.java.net/~ihse/JDK-8142909-hotspot-build-infra-integration/webrev.03

I have dropped all the changes related to -g and OPT_CFLAGS. I'm not 
interested in that kind of large scale operations on the old build 
system, I just wanted to transfer the small patches we have in the 
build-infra forest to mainline to facilitate comparison between old and 
new build. If that was too problematic, let's skip it.


Remaining changes are:
 * Making adlc more quiet
 * Sorting the windows build

Also, in the meantime, a new difference has surfaced. In the first push 
of the new hotspot build system, the rewrite of SA, the file 
make/bsd/makefiles/saproc.make unfortunately was not deleted in 
mainline. I added the deletion of this file.


I intend to push this through hs-rt. David and Erik, please confirm that 
these changes are OK for you.


/Magnus


On 2015-11-17 02:57, David Holmes wrote:

On 16/11/2015 10:14 PM, Erik Joelsson wrote:

On 2015-11-16 12:11, David Holmes wrote:

On 16/11/2015 6:22 PM, Erik Joelsson wrote:

On 2015-11-16 03:10, David Holmes wrote:

Hi Magnus,

On 13/11/2015 9:33 PM, Magnus Ihse Bursie wrote:

On 2015-11-13 09:13, Erik Joelsson wrote:

Hello,

make/bsd/makefiles/amd64.make:
make/bsd/makefiles/gcc.make:
Perhaps the order of "$(OPT_CFLAGS/NOOPT) 
$(OPT_CFLAGS/$(BUILDARCH))"

should be reversed to guarantee that NOOPT is the one used in case
BUILDARCH contains something that conflicts? The solaris file 
does it

this way.

You know you wrote that code originally? ;-)

Yeah, I agree, it's more reasonable. Updated webrev:
http://cr.openjdk.java.net/~ihse/JDK-8142909-hotspot-build-infra-integration/webrev.02 






I don't like this particular change as it is too generic. It gives 
the

appearance of being apply to apply whatever the BUILDARCH specific
optflags are, which makes no sense if the whole point here is to not
optimize these files. If all this is intended to do is include -g 
then

I think that should be done explicitly.


The -g flag is added to OPT_CFLAGS/$(BUILDARCH) and I can only assume
the intention was to have it added to all compilation command lines.
However, the OPT_CFLAGS/ variable overrides
CFLAGS/$(BUILDARCH), effectively removing the -g flag from the files
where we explicitly change the optimization level. The patch 
intends to

make sure -g is used on those files too.


I understand that, but it is far from obvious that OPT_FLAGS/BUILDARCH
means -g (why???), or that putting other things in that variable might
break files that should not be optimized.


A cleaner solution would be to not have -g be part of the OPT flags at
all but its own set of DEBUG_CFLAGS.


I thought -g was in the DEBUG_CFLAGS ?? Or maybe it used to be. Anyway
if all we are interested in is adding -g then I agree it should have
its own variable.


I looked some more at this. There is a DEBUG_CFLAGS, FASTDEBUG_CFLAGS
and OPT_CFLAGS. These get picked depending on the debug level of the
build. For example in product.make, $(OPT_CFLAGS/BYFILE) gets added to
CFLAGS.

I can't really see a better way of making sure the -g does not fall off
for some files than what is proposed here. At least not without
completely reworking the flags handling in the current hotspot
makefiles, something I'm very uninterested in doing.


In the -g case what we are handling is the addition of -g to product 
builds (it is already on fastdebug, debug) when 
ENABLE_FULL_DEBUG_SYMBOLS is selected, on a subset of architectures. 
As a convenience we define it thus (BSD version):


OPT_CFLAGS/ia64 = -g
OPT_CFLAGS/amd64 = -g
OPT_CFLAGS/arm = -g
OPT_CFLAGS/ppc = -g
OPT_CFLAGS += $(OPT_CFLAGS/$(BUILDARCH))

so $(OPT_CFLAGS/$(BUILDARCH)) happens to contain -g. I would be happy 
if the above were changed to:


FDS_CFLAGS/ia64 = -g
FDS_CFLAGS/amd64 = -g
FDS_CFLAGS/arm = -g
FDS_CFLAGS/ppc = -g
OPT_CFLAGS += $(FDS_CFLAGS/$(BUILDARCH))

and then you could use $(FDS_CFLAGS/$(BUILDARCH) on the "by-file" 
definitions, where it would be much clearer what kind of flag is 
actually being applied. However this needs to be done everywhere, and 
I don't see this fix attempting to do that. We set 
OPT_CFLAGS/ in numerous places (open and closed) to regain 
control over the "opt" flags applied to that file. Sometimes we use 
predefined opt flags, like NOOPT or SPEED, and sometimes we don't eg:


linux/makefiles/amd64.make

OPT_CFLAGS/compactingPermGenGen.o = -O1

This would need to be applied to every occurrence of the "byfile" 
definitions in all the makefiles. Or else ascertained on a 
case-by-case basis that we don't want -g for a specific file.


Further, on architectures where $(FDS_CFLAGS/$(BUILDARCH) is not set 
you still have to somehow handle the rest of the above logic:


 507 ifeq ($(OPT_CFLAGS/$(BUILDARCH)),)
 508   ifeq ($(USE_CLANG), true)
 509 # Clang doesn't understand -gstabs
 510 OPT_CFLAGS += -g
 511   else
 512 OPT_CFLAGS