Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARM builds for OpenJDK #686

Merged
merged 2 commits into from
Dec 10, 2017
Merged

Conversation

JamesKingdon
Copy link
Contributor

Added a new buildspec and updated the build scripts to work with the new Docker image for OpenJDK builds.

@JamesKingdon
Copy link
Contributor Author

@jdekonin Perhaps you could review these changes that go along with those in ibmruntimes/openj9-openjdk-jdk9#88 ?

Add a Dockerfile with the toolchain and native libraries for
cross compiling for armhf.

Signed-off-by: James Kingdon <jkingdon@ca.ibm.com>
Copy link
Contributor

@jdekonin jdekonin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with these changes. I am not 100% sold on the need of OPENJ9_CC_PREFIX, but I don't have an alternative option to suggest and I understand how its usage is required. Note I am not an openj9 committer, so this will need additional eyes on, but it doesn't need to be blocked by ibmruntimes/openj9-openjdk-jdk9#88

# During openJDK build, SPEC arrives as
# /root/openj9-openjdk-jdk9/build/linux-arm-normal-server-release/spec.gmk
# and PLATFORM is not set, so the above doesn't work.
ifdef OPENJ9_CC_PREFIX
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should be depending on something defined by the environment, better to (indirectly) depend on something declared by the buildspec which can be converted into an export from mkconstants.mk.ftl

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or actually runtime/compiler/makefile.ftl

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @pshipton , it's an interesting point. The details of the cross compiler tool chain are known in (and indeed set by) the Dockerfile, so it seems natural to push that information from the Dockerfile into the build. Any mechanism that doesn't take the information from the Dockerfile is clearly both more work and an opportunity to introduce an error. Using environment variables defined by the Dockerfile is simply the most obvious way of passing the information from Docker to the build, but I guess we could do it any number of other ways. Do you object specifically to using env vars or more generally to passing the information from Docker?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps I'm too stuck on old ways of doing things. In the future there will be a configure step that takes settings from the environment or by parameter, even though up to now its been controlled by the build specs. Although to do this correctly it seems flags like -march=armv6 -marm -mfpu=vfp -mfloat-abi=hard must also be derived from the OPENJ9_CC_PREFIX.

How does the OpenJDK compile determine which versions of the tools to use?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems flags like -march=armv6 -marm -mfpu=vfp -mfloat-abi=hard must also be derived from the OPENJ9_CC_PREFIX

Not quite, a given toolchain (as defined by prefix) can build for many arch targets, and indeed I have another PR in the works for switching between armv6 and armv7. In the future we may also need to support v5 for embedded applications, aarch64 for servers and softfloat for some subset of the above.
OpenJDK hasn't (yet?) embraced Docker files, so the tool chain has to be specified via options to their configure script, e.g.

bash ./configure --openjdk-target=arm-linux-gnueabihf --with-abi-profile=arm-vfp-hflt --with-x=../../gcc-linaro-4.9.4-2017.01-x86_64_arm-linux-gnueabihf/arm-linux-gnueabihf/ --with-freetype=/home/jkingdon/tr/openJDK/gcc-linaro-4.9.4-2017.01-x86_64_arm-linux-gnueabihf/arm-linux-gnueabihf/libc/usr/

I opened #518 to address better integration with the open jdk configure options, and would much prefer to address that work there rather than blocking a working ARM build.

I created a new build spec as part of this set of PRs (this is only one of many required to get the build working on ARM), and it seems to me that adding new specs is a considerable amount of work with the potential for introducing errors. I'd be hesitant about any solution that is based on a ballooning number of build-specs.

One possibility for passing configuration information between Docker and the build is to add scripts to the image that contain the relevant info. For example, that long winded configure commandline could be wrapped in a script and packed in the Docker image. This would make more sense after #518 has been addressed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and would much prefer to address that work there rather than blocking a working ARM build

+1

# /root/openj9-openjdk-jdk9/build/linux-arm-normal-server-release/spec.gmk
# and PLATFORM is not set, so the above doesn't work.
ifdef OPENJ9_CC_PREFIX
PLATFORM=arm-linux-gcc-cross
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly the platform shouldn't be assumed, you can update runtime/compiler/makefile.ftl to set it

else
# Override the GLOBAL_INCLUDES value provided by configure_common.mk
CONFIGURE_ARGS += 'GLOBAL_INCLUDES=/bluebird/tools/arm/arm-bcm2708/arm-bcm2708hardfp-linux-gnueabi/arm-bcm2708hardfp-linux-gnueabi/include'
CONFIGURE_ARGS += 'AS=bcm2708hardfp-as'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it might make more sense to provide a default for OPENJ9_CC_PREFIX (bcm2708hardfp) so AS, CC, etc. can be added to configure args just one way each.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'd much prefer the approach you suggest too. I was trying to be ultra cautious about breaking existing behaviour, but I think the benefits of your suggestion outweigh the risks of getting it wrong.

ifdef OPENJ9_CC_PREFIX
OBJCOPY := $(OPENJ9_CC_PREFIX)-objcopy
else
OBJCOPY := bcm2708hardfp-objcopy
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, this could be simpler as:

OPENJ9_CC_PREFIX ?= bcm2708hardfp
OBJCOPY := $(OPENJ9_CC_PREFIX)-objcopy

<defaultSizes>desktop (256M + big OS stack + growable JIT caches)</defaultSizes>
<priority>100</priority>
<owners>
<owner>aaron_rocha@ca.ibm.com</owner>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove/replace this email. Aaron doesn't work for IBM any more.

SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
-->

<spec xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://www.ibm.com/j9/builder/spec" xsi:schemaLocation="http://www.ibm.com/j9/builder/spec spec-v1.xsd" id="linux_arm_docker">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be named "docker". Docker isn't a requirement, just a way to set up an appropriate environment.

</source>
<flags>
<flag id="arch_arm" value="true"/>
<flag id="build_autobuild" value="true"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

build_autobuild and build_java8 also need to be removed or set to false.

@JamesKingdon JamesKingdon force-pushed the arm-buildspec branch 2 times, most recently from a5ea1a8 to 5367b3a Compare December 7, 2017 02:10
@JamesKingdon
Copy link
Contributor Author

@keithc-ca @pshipton I think these changes address everything discussed, excepting the work that I think we agreed was better handled in #518

@@ -0,0 +1,136 @@
# Copyright (c) 2017, 2017 IBM Corp. and others
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To match the other platforms, this Dockerfile should be in a ubuntu16 subdirectory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about this on the way to work this morning, I'm not sure ubuntu16 is correct. For other platforms ubuntu16 is the OS where the binary is built and where it runs. Since arm is cross-compiled, perhaps arm-linux-gnueabihf is more appropriate.


# To use this docker file:
# docker build -t=openj9arm .
# docker run -v </path/openj9-openjdk-jdk9>:/root/openj9-openjdk-jdk9 -it openj9arm
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other platforms don't suggest adding a volume.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I don't understand that. We should open an issue to have the other platforms fixed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should open an issue...

I agree. But until there is agreement on it and we change all the plats to be consistent I think this one should match the others.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whether the others are 'broken' is a matter of perspective. Only if you want changes persisted on the host are the volume options needed.

@@ -21,7 +21,16 @@
-->

<#if uma.spec.processor.arm>
OBJCOPY := bcm2708hardfp-objcopy
ARM_ARCH_FLAGS := -march=armv6 -marm -mfpu=vfp -mfloat-abi=hard
<#--
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this comment be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely, it's an oversight. Thanks for pointing it out.

<property name="sun.jdk7.platform_id" value="linux-arm"/>
<property name="sun.jdk8.platform_id" value="linux-arm"/>
<property name="svn_stream" value=""/>
<!-- cross compiler path not needed with Docker image -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its the environment setup that obsoletes the need for the cross compiler path, not Docker.

@JamesKingdon
Copy link
Contributor Author

@pshipton comments addressed.

Added a new buildspec that makes use of the Docker image for
cross compiling OpenJDK for ARM. Updated build scripts to match.

Signed-off-by: James Kingdon <jkingdon@ca.ibm.com>
# [2] http://openjdk.java.net/legal/assembly-exception.html
#
# SPDX-License-Identifier: EPL-2.0 OR Apache-2.0

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The instructions to run were completely removed. I'm going to merge anyway and they can be added afterwards.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the merge.
I feel quite strongly about the use of volumes when using Docker for iterative development (it's less of an issue for automated build), but my realisation was that I was about to go to war over a comment. The Dockerfile has no dependency one way or another, so any comment about how to use it within the file is little more than a suggestion (but with the potential to confuse). Since the usage can vary depending on context I would recommend not putting that information inside the Dockerfile, but keeping it in the external documentation, which can be specialised accordingly.

@pshipton pshipton merged commit 219790f into eclipse-openj9:master Dec 10, 2017
@jabrena
Copy link

jabrena commented Jan 9, 2018

@JamesKingdon
Copy link
Contributor Author

Looks great @jabrena What's the status, does it run with the JIT?

@jabrena
Copy link

jabrena commented Jan 10, 2018

Good morning @JamesKingdon,

Yes, I am very happy with the contribution of @JakubVanek because he unblocked the usage of Java 9 in the Lego Mindstorms EV3 platform and in the last weeks I was pretty busy with others POCS in Java ecosystems (Kafka Streams) but I would like to compare results with OpenJ9 to be fair and I would like to execute the benchmark for all cases:
https://github.com/ev3dev-lang-java/jvm-benchmark

This JVM doesn't provide a JIT:

robot@brick1:~/jre-ev3/bin$ ./java -version  
openjdk version "9.0.1"
OpenJDK Runtime Environment (build 9.0.1+11)
OpenJDK Client VM (build 9.0.1+11, mixed mode)

So, don't worry OpenJ9 should be a real alternative for EV3 in 2018, but little by little. :)

In the next weeks, I will automate the installation of a JVM on EV3:
https://github.com/ev3dev-lang-java/installer/blob/master/modules/java.sh

Juan Antonio

@JakubVanek
Copy link

Hi,
OpenJDK 9 has JIT for ARM32, otherwise there would be 'interpreted mode' instead of 'mixed mode'. It is the same compiler as in the closed-source EJDK8, as it was released to OpenJDK.
I've just tried changing the build flags enough times for the build to succeed.
Jakub Vaněk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants