-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
8303796: Optionally build fully statically linked JDK image #13709
Conversation
- Rename ALL_STATIC_LIBS to JDK_STATIC_LIBS. - Define JVM_STATIC_LIB as $(STATIC_LIBS_IMAGE_DIR)/lib/$(LIBRARY_PREFIX)jvm$(STATIC_LIBRARY_SUFFIX). Add $(JVM_STATIC_LIB) to TARGETS. - Remove unused STATIC_LAUNCHER_IMAGE_DIR and STATIC_LAUNCHER_IMAGE_SUBDIR from make/autoconf/spec.gmk.in.
/contributor add Man Cao manc@google.com |
👋 Welcome back jiangli! A progress list of the required criteria for merging this PR into |
@jianglizhou |
@jianglizhou The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
Webrevs
|
@jianglizhou I thought this work was proposed to be done under the Project Leyden umbrella? The other static-build tweaks have been fine but this seems to be part of a bigger, yet unspecified, project that may need to be covered by a JEP. |
The JDK moved to modular run-time images in JDK 9 (JEP 220) that are created with the jlink tool. The eventual goal here is to make it possible to produce a statically linked image and I suspect there will need to be some figuring out as to where jlink fits. It might be that the JDK produces a set of packaged modules containing the .a files and jlink involves the native linker. I think some future JEP will need describe all this. If I understand the make file changes proposed here, it runs the native linker to create "javastatic" with the launcher, libjvm and other JNI libs linked into one executable, this is generated and copied into the run-time image created by jlink. I think this will need discussion as its more like an overlay. I think it is useful to have the build create the .a files as that's the first step towards putting them into packaged modules for producing static images. |
The build is already capable of producing .a files and this patch is building on top of that build feature. The current .a file creation is used by the downstream graal build which needs it for nativeimage. |
I pulled this PR and had a go at building it. For me it failed with errors like this:
I haven't dug any deeper to try to figure this out. Is this something you recognize? |
FREETYPE_TO_USE=system | ||
fi | ||
if test "x$with_freetype" != "x" ; then | ||
if test "x$with_freetype" = "xsystem" ; then | ||
if test "x${STATIC_JAVA}" = "xtrue"; then | ||
AC_MSG_ERROR([--with-freetype=system does not work with --with-static-java=yes]) | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not ? Surely you aren't statically linking every library you find on the platform that's referenced by JDK ?
I mean its fine to say that I'd prefer to statically link this library, but I don't understand the claim that this is the only thing that can work ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not ? Surely you aren't statically linking every library you find on the platform that's referenced by JDK ?
I mean its fine to say that I'd prefer to statically link this library, but I don't understand the claim that this is the only thing that can work ?
It's for providing libzip.a and libfreetype.a as part of the JDK 'static-libs' image. In our usages, I've learned that static linking is preferred whenever possible. We don't statically link libc.
Mine main reasoning/motivation for building and including the libzip.a and libfreetype.a in JDK 'static-libs' image is for ease of use. It reduces complications during the post hermetic image build process, if those libraries are need.
# static libraries cause linking errors due to duplicate symbols. | ||
LIBAWT_XAWT_STATIC_EXCLUDE_OBJS := \ | ||
debug_assert.o debug_util.o debug_trace.o debug_mem.o systemScale.o | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Umm. Does this mean you are linking both headless and xawt into the same image ?
You are on your own with that. Sounds like a mistake to me and I don't want to see any
proposed source code changes to fix up any issues you see later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Umm. Does this mean you are linking both headless and xawt into the same image ?
For the xawt case, we statically link with libawt.a and libawt_xawt.a in our early prototype. For headless case, we statically link with libawt.a and libawt_headless.a. We have been focusing on testing with the headless usages.
The libawt provides AWT_OnLoad
, which handles the 'loading' of libawt_xawt or libawt_headless. By filtering out the duplicate .o files in the static linking case, it can work without larger refactoring the sources in the those libraries.
Hi David, indeed the hermetic project was proposed under Project Leyden umbrella. The makefile changes are related to the static linking only, e.g.:
That gives us a base for exposing/resolving needed runtime work for supporting static linking fully in JDK. Either covering the makefile work by a JEP or handling it as an incremental change without JEP sounds good. The later might fit the current purpose better. Updated: Reading through the other comments, @erikj79 also suggested a JEP would be needed in this case. I'll move forward creating a JEP after collecting some additional input. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the other comments here saying that this looks like something that would need a JEP. Especially since this isn't the full feature, but just the first step, and is essentially putting in dead code. It also seems like a higher level discussion about the solution may be needed.
Independent of the above, I have done a brief review of the code as it is.
@@ -1049,6 +1064,8 @@ else | |||
|
|||
static-libs-image: $(STATIC_LIBS_TARGETS) | |||
|
|||
static-java-image: jdk-image hotspot-server-static-libs static-libs-image |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't need to repeat this. The DEPS parameter to SetupTarget should have handled it.
################################################################################ | ||
AC_DEFUN_ONCE([STATIC_JAVA_SETUP], | ||
[ | ||
AC_ARG_WITH(static-java, [AS_HELP_STRING([--with-static-java], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is all the setup for static-java in configure, I don't think it warrants having its own file. I would put this in jdk-options.m4 for now.
LIBJLI_STATIC_EXCLUDE_OBJS := \ | ||
inflate.o inftrees.o inffast.o zadler32.o zcrc32.o zutil.o |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the same list as the LIBJLI_EXTRA_FILES above. Would be good to avoid the duplication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LIBAWT_XAWT_STATIC_EXCLUDE_OBJS := \ | ||
debug_assert.o debug_util.o debug_trace.o debug_mem.o systemScale.o |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this list be derived dynamically in some way? If they are all in the same directory, maybe we could base it on that instead of having to maintain an explicit list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With additional testing, I found 'debug_assert.o debug_util.o debug_trace.o debug_mem.o' are no longer included when building libawt_xawt, using JDK head code base. We no longer need to be filtered out those for libawt_xawt.a. They were needed for JDK 11.
$(JAVASTATIC_TARGET_IMAGE): $(JAVASTATIC_LINK_LIBS) | ||
ifeq ($(call isTargetOs, linux), true) | ||
$(ECHO) "Creating '$(JAVASTATIC_TARGET_IMAGE)'" | ||
$(call ExecuteWithLog, $(JAVASTATIC_LOG_PREFIX), \ | ||
$(LD) $(JAVASTATIC_LDFLAGS) $(LD_OUT_OPTION)$(JAVASTATIC_TARGET) \ | ||
$(JAVASTATIC_LD_OBJ_ARG) $(JAVASTATIC_LIBS) $(JAVASTATIC_EXTRA_LIBS)) | ||
$(CP) -f $(JAVASTATIC_TARGET) $@ | ||
else | ||
$(error Not supported on non-linux platform currently) | ||
endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be rather beneficial if this linking step could be performed by some variant of SetupNativeCompilation. We don't want to have to maintain logic for dealing with different platforms and linkers outside of the common files if it can be helped. SetupNativeCompilation would need to be tweaked to run without SRC and just a set of .a
files.
The error about platform support should be handled in configure. We shouldn't be able to get here unless the configuration is expected to be supported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be rather beneficial if this linking step could be performed by some variant of SetupNativeCompilation. We don't want to have to maintain logic for dealing with different platforms and linkers outside of the common files if it can be helped. SetupNativeCompilation would need to be tweaked to run without SRC and just a set of .a files.
The error about platform support should be handled in configure. We shouldn't be able to get here unless the configuration is expected to be supported.
All sounds good, thanks. I plan to withdraw this PR and extract the .a handling part into a new PR for now (as mentioned in response to @AlanBateman's comments). For the linking part, it would be good to have your close/direct involvement and collaboration in branch.
Erik, could you please share your
|
I can't see any significant difference. I'm using a devkit created using the devkit makefiles.
|
For the testing/demo purpose (on static linking part) with this PR, the makefile changes in the PR only run the native linker step to perform static linking. The 'javastatic' is simply copied into the (regular) jlink'ed JDK image 'bin/' directory. In our experiments/prototyping work, the 'javastatic' launcher binary is used along with the other artifacts from the regular JDK image (e.g. lib/modules) for basic sanitary testing (running some of the jtreg tier 1 tests). The 'javastatic' is not used for creating the hermetic Java image. The post process building the hermetic image takes the .a files, lib/modules and other resources files as input.
Agreed. Based the initial feedback from you and other reviewers in the thread, I'll repurpose this PR for handling the .a part only. I'll split the 'javastatic' static linking part into a separate branch for needed discussions (including the potential JEP part). That branch can be on the sideline and it can be to verify/test the other static linking related changes. |
On the second thought, it might be cleaner to withdraw the current PR first and extract the .a part into a new PR. |
I did some search and found https://stackoverflow.com/questions/2418157/c-error-undefined-reference-to-clock-gettime-and-clock-settime/32649327#32649327. The clock_* functions moved from librt to libc (starting from glibc 2.17). That finding led me to the following in libraries.m4:
Since I use |
Based on the above finding, I pushed a change to use $(JVM_LIBS) for the linking command. @erikj79 could you please see if that resolves the clock_* symbol issues in your environment? |
Also builds on recent changes to remove duplicate symbol names. It's good that the PR demonstrates that these can be linked to create "javastatic" but having it copy into the run-time image created by jlink is probably not the eventual outcome. A possible direction on this is for the build to create a set of packaged modules with the .a files, then have an alternative image builder in jlink that integrates with the native linker. Combined with other parts in Jiangli's slides/proposal, it would mean that jlink could produce a single executable that would work for both the JDK or any set of modules. |
It seems that letting jlink do the linking is also a requirement for user provided modules (.jmods) that contain native libraries (as is the case for instance with jextract). We don't know about those libraries when building the JDK. |
For now yes, but if there are further steps to have it create a single executable then the classes/resources will also be included. So I think pure Java modules will also be in this picture, eventually. |
Yes, it built cleanly with that change. |
The comment from @AlanBateman helped me realize that I should be more specific (in my earlier comment regarding "withdraw the current PR first and extract the .a part into a new PR") about the needed fixes/enhancements related to .a part:
It's probably best to create a separate enhancement/bug for those (will create shortly) and continue using JDK-8303796 for the broader discussion/effort. |
Thanks for confirm. |
Created https://bugs.openjdk.org/browse/JDK-8307194 for the need static library (.a handling) enhancements. |
@jianglizhou this pull request can not be integrated into git checkout JDK-8303796
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push |
…inked JDK build, i.e. building java launcher executable statically linked with JDK and hotspot native code. To build static JDK: - bash configure --with-boot-jdk=<jdk_path> --with-static-java=yes - make static-java-image
Created https://github.com/jianglizhou/jdk/tree/static-java branch, which includes the makefile changes from this (closed) PR for building a fully statically linked java launcher executable to demonstrate the JDK static support. |
…ly linked JDK build, i.e. building java launcher executable statically linked with JDK and hotspot native code. To build static JDK: - bash configure --with-boot-jdk=<jdk_path> --with-static-java=yes - make static-java-image
Initial implementation for supporting building a fully statically linked (with a desired set of JDK native libraries and libjvm) Java launcher executable, which is named as 'javastatic'.
In this PR, the support is only added for the linux platform. Both gcc and clang can be supported. For current demo/testing purpose, the bin/javastatic is statically linked with awt headless and other common JDK native libraries. The current PR doesn't fully handle creating the bundle for a static JDK image, which can be supported later.
To build the statically linked executable:
The 'javastatic' binary created by the static-java-image target is not runnable. The runtime issues will be handled separately.
Following is a summary of the changes in this PR:
Add make/autoconf/static-java.m4 for defining STATIC_JAVA_SETUP. Add STATIC_JAVA_SETUP to make/autoconf/configure.ac.
Changes in make/Main.gmk
- Add HOTSPOT_VARIANT_STATIC_LIBS_TARGETS and DeclareHotspotStaticLibsRecipe for building libjvm static library.
- Add static-java-image for creating the fully statically linked standard Java launcher binary, bin/javastatic. The build process also places libjvm.a into the 'static-libs' image lib/ directory.
Add make/StaticLink.gmk, which contains the main support for creating the fully statically linked Java launcher binary.
Setup LDFLAGS_CXX_STATIC_JDK based on $TOOLCHAIN_TYPE in make/autoconf/flags-ldflags.m4.
Always use bundled libraries for zlib, freetype, etc for static build support. The related changes are in make/autoconf/lib-bundled.m4 and make/autoconf/lib-freetype.m4. Building the bundled zlib, freetype and etc libraries ensures those libraries are included in the JDK binary bundle. It decouples the assumptions/requirements of the static Java image build process from the assumptions/requirements of the JDK build process. A post process that builds the static Java image can use those bundled libraries provided by JDK binary if needed.
When building a fully statically linked java launcher executable, the --whole-archive linker option is used for the JDK/VM static libraries to make sure it links every object (.o) file provided by those static libraries. As a result, we need to remove any duplicate object files from the different JDK/VM static libraries. To do that, STATIC_LIB_EXCLUDE_OBJS is added and used in make/common/NativeCompilation.gmk. STATIC_LIB_EXCLUDE_OBJS contains the list of object files that need to be filtered out when creating a specific static library. STATIC_LIB_EXCLUDE_OBJS is defined for JDK/VM static libraries that may contain object files from other libraries (those are needed when building shared libraries), and those object files are added to the STATIC_LIB_EXCLUDE_OBJS. See make/hotspot/lib/CompileJvm.gmk, make/modules/java.base/lib/CoreLibraries.gmk and make/modules/java.desktop/lib/Awt2dLibraries.gmk.
In make/common/NativeCompilation.gmk, move the code handling long arguments so that it can be used for the static build support as well.
In make/hotspot/lib/CompileJvm.gmk, it specifies to exclude operator_new.o from the libjvm static library. See details in the comment added in CompileJvm.gmk.
Thanks manc for a bug fix for JAVASTATIC_OBJECT_DIR in StaticLink.gmk.
Progress
Integration blocker
Issue
Contributors
<manc@google.com>
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/13709/head:pull/13709
$ git checkout pull/13709
Update a local copy of the PR:
$ git checkout pull/13709
$ git pull https://git.openjdk.org/jdk.git pull/13709/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 13709
View PR using the GUI difftool:
$ git pr show -t 13709
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/13709.diff
Webrev
Link to Webrev Comment