8255240: Mobile builds need to exclude some modules #10

johanvos · 2020-10-22T12:58:48Z

exclude some modules that are not relevant on Mobile.
Fix for JDK-8255240

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8255240: Mobile builds need to exclude some modules (Task - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/mobile.git pull/10/head:pull/10
$ git checkout pull/10

Update a local copy of the PR:
$ git checkout pull/10
$ git pull https://git.openjdk.org/mobile.git pull/10/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 10

View PR using the GUI difftool:
$ git pr show -t 10

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/mobile/pull/10.diff

Webrev

Link to Webrev Comment

Fix for JDK-8255240

bridgekeeper · 2020-10-22T12:59:39Z

👋 Welcome back jvos! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

mlbridge · 2020-10-22T13:04:18Z

Webrevs

bobvandette · 2020-10-22T13:20:29Z

Is this a temp fix? Excluding SA certainly makes sense but eliminating the entire desktop module will prohibit the use of local imaging and text APIs. Shouldn't you focus more on eliminating the native windowing toolkit?

johanvos · 2020-10-22T13:33:05Z

We (at Gluon) only use the static libraries from the mobile builds, not the modules/jars (they come from the upstream builds).
Hence, the desktop module (which also contains the beans etc) is still used, but the binary (jmod) comes from upstream.

However, it would make sense to build the module here too, and only exclude the native part indeed.

Don't build the native sound/window libs though. Add changes for generating char files and Xevent files (android)

johanvos · 2020-10-23T09:46:40Z

@bobvandette java.desktop is back in the list (at least the module will be build).

johanvos · 2020-10-24T10:21:06Z

@magicus Do you want to have a look at this PR too?

bobvandette · 2020-10-24T13:11:43Z

make/modules/java.desktop/Lib.gmk

@@ -34,12 +34,14 @@ $(call FillFindCache, $(wildcard $(TOPDIR)/src/java.desktop/*/native))
 ################################################################################
 # Create the AWT/2D libraries

-include lib/Awt2dLibraries.gmk
+ifeq ($(call isTargetOs, android ios), false)
+  include lib/Awt2dLibraries.gmk


This change seems too extreme. You'll have access to the desktop module but any use of the java code in this module will fail since libawt will be missing. The real fix is to configure the sources and makefiles so the entire code path java.desktop -> native libraries is available but only for the headless toolkit. The libawt_lwawt should not be included. This may be difficult since the fontmanager is dependent this native code.

If this is too difficult, you should just go back and not support the java.desktop module like you had in your first PR version. Although, every time you add a change like this which moves away from full JRE JCK compatibility, you make it more difficult to upstream these changes.

It would be possible to fix configure so mobile always use headless mode. From my PoV, it seems like the natural approach. Let me know if you need help with fixing that.

The problem with awt_headless is that this depends on X11 (which we don't have on iOS/Android).
I'm not sure what the best approach is. It would need lots of changes to make that work on mobile.
Any help is much appreciated.

Now that you mention it, I do remember some fuss long time ago about headless requiring X11. Hmmm. That means that any attempt to use headless is definitely out of bounds for this patch. Maybe the best way for this patch is then to exclude the java.desktop module for iOS and Android in Modules.gmk.

But I'll look into headless once more to see if I can figure out if it really needs X11 or if it's just some sloppy programming.

Good news! It turned out that java.desktop can build just fine without X11 if you just want headless.

A quick and dirty hack to confirm that this works for you too:

diff --git a/make/autoconf/libraries.m4 b/make/autoconf/libraries.m4 index 5120918aed2..42a641c3228 100644 --- a/make/autoconf/libraries.m4 +++ b/make/autoconf/libraries.m4 @@ -46,7 +46,7 @@ AC_DEFUN_ONCE([LIB_DETERMINE_DEPENDENCIES], else # All other instances need X11, even if building headless only, libawt still # needs X11 headers. - NEEDS_LIB_X11=true + NEEDS_LIB_X11=false fi # Check if fontconfig is needed

I'll open a separate bug to get a fix in mainline so that X11 is not required for headless builds.

Maybe try add missing includes before exclude?

I'd like to include as much as possible, but if that results in ending up complete implementations between
#ifndef IOS
#endif
statements, I worry that we are moving away from the platform-independent idea as well.

The GLX code is a problem since glx.h has an include to X11/Xlib.h and X11/Xutil.h and it really depends on what is defined there. That header is included in e.g. GLXGraphicsConfig.c (via GLXGraphicsConfig.h)

There is a check on HEADLESS in that file, but it's after the includes. I can move that #ifdef before the includes, but that basically rules out the whole file, and then we are closer to the scenario were everything is included, but all implementations are removed via #ifndef IOS (or #ifndef ANDROID)

I ran into a small build issue, where the -DHEADLESS was not passed to LIBAWT_CFLAGS but that is an easy fix.

I think the include of GLXGraphicsConfig.h should really be inside the ifdefs. It does not make sense as it is now. If you look at the GLX code, it is basically split in two, a "dummy" implementation for headless which just does:

JNIEXPORT jint JNICALL Java_sun_java2d_opengl_GLXGraphicsConfig_getOGLCapabilities(JNIEnv *env, jclass glxgc, jlong configInfo) { return CAPS_EMPTY; }

and the real implementation for non-headless.

But this got me worried about my fix of building headless without X11. I wonder how I could compile this without X11, or if the compiler dragged up X11 from a local installation without me noticing it. :(

I fixed the issues, and with the latest commit, the awt libs are built for ios/android.

bobvandette · 2020-10-24T13:14:47Z

make/common/Modules.gmk

@@ -231,6 +231,16 @@ ifeq ($(call isTargetOs, windows macosx linux), false)
  MODULES_FILTER += jdk.incubator.jpackage
 endif

+################################################################################
+# Filter out specific modules for mobile platforms


Is there a reason why you want to excluded these modules? I would think that it might be nice to be able to manage a mobile application remotely. If these modules are incompatible with native-image, then just remove them from your built images when you create a GraalVM builder image.

They are included again now.

But jdk.sctp is still left out on ios?

Why don't just exclude GLX (Desktop OpenGL), or port to EGL with OpenGL ES because both Android and iOS does not have Desktop OpenGL (althought GL4ES is a solution).

Excluding GLX would be good. I'm looking into that, but there are other files that need to be excluded as well (java2d/x11).
The port to EGL/OpenGL ES is exactly what is done in the OpenJFX project, and therefore it seems a bit redundant to me to re-introduce that here.

correct, that one is still failing on ios (Sctp.h includes linux/types.h)

@johanvos If you look at make/modules/jdk.sctp/Lib.gmk, you see that the native library is excluded on macosx and aix.

I'm not sure about the details here, maybe sctp support is not possible on iOS, but maybe the macos pure java implementation is what's needed on iOS as well?

Heh. I just had a look at the macos implementation, or should I say "implementation". It's basically:

private static final String message = "SCTP not supported on this platform"; public SctpChannelImpl(SelectorProvider provider) { super(provider); throw new UnsupportedOperationException(message); }

so I dunno, maybe it's just as well to skip the entire module.

magicus · 2020-10-26T08:40:44Z

make/autoconf/jdk-options.m4

+  if test "x$OPENJDK_TARGET_CPU" = xandroid ; then
+    INCLUDE_SA=false
+  fi
+  if test "x$OPENJDK_TARGET_CPU" = xios ; then


You mean OPENJDK_TARGET_OS, android/ios are not CPUs. :-)

Oops, indeed. Fixed.

magicus · 2020-10-26T08:42:50Z

make/common/Modules.gmk

+# Filter out specific modules for mobile platforms
+
+ifeq ($(call isTargetOs, ios android), true)
+  MODULES_FILTER += jdk.hotspot.agent


You don't need to do this since you turn off SA. There is already a
ifeq ($(INCLUDE_SA), false) MODULES_FILTER += jdk.hotspot.agent endif

magicus · 2020-10-26T08:44:15Z

make/common/Modules.gmk

+  MODULES_FILTER += jdk.hotspot.agent
+  MODULES_FILTER += jdk.management
+  MODULES_FILTER += jdk.management.agent
+  MODULES_FILTER += jdk.sctp


While technically correct, I agree with Bob that there seem to be not much point in disabling these, if the reason is just "don't think we need them". Is there an issue with compiling these modules on mobile?

The compile issues were minimal, and they are solved now.

magicus · 2020-10-26T08:45:50Z

make/common/modules/LauncherCommon.gmk

@@ -51,7 +51,7 @@ LAUNCHER_CFLAGS += -I$(TOPDIR)/src/java.base/share/native/launcher \

 ifeq ($(call isTargetOs, ios), true)
  LAUNCHER_CFLAGS += -I$(TOPDIR)/src/java.base/macosx/native/libjli
-fi
+endif


Did the original file in mobile have a broken if statement? So this is like a separate bug fix? Otherwise I don't understand what I'm seeing here.

That was indeed broken in the mobile build.

magicus · 2020-10-26T08:47:35Z

make/modules/java.desktop/Lib.gmk

@@ -34,12 +34,14 @@ $(call FillFindCache, $(wildcard $(TOPDIR)/src/java.desktop/*/native))
 ################################################################################
 # Create the AWT/2D libraries

-include lib/Awt2dLibraries.gmk
+ifeq ($(call isTargetOs, android ios), false)
+  include lib/Awt2dLibraries.gmk


It would be possible to fix configure so mobile always use headless mode. From my PoV, it seems like the natural approach. Let me know if you need help with fixing that.

khanhduytran0 · 2020-11-01T00:26:21Z

Excuse me, but why don't you port jdk8 instead?
Also, I succeed to port jdk8 to Android, but stuck at

/home/runner/work/android-openjdk-build-multiarch/android-openjdk-build-multiarch/openjdk/jdk/src/share/native/sun/java2d/loops/GraphicsPrimitiveMgr.c: In function 'InitSimpleTypes.constprop':
/home/runner/work/android-openjdk-build-multiarch/android-openjdk-build-multiarch/openjdk/jdk/src/share/native/sun/java2d/loops/GraphicsPrimitiveMgr.c:314:17: internal compiler error: in build2_stat, at tree.c:4159
 static jboolean InitSimpleTypes
                 ^
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://source.android.com/source/report-bugs.html> for instructions.
make[2]: *** [/home/runner/work/android-openjdk-build-multiarch/android-openjdk-build-multiarch/openjdk/build/linux-aarch64-normal-server-release/jdk/objs/libawt/GraphicsPrimitiveMgr.o] Error 1
lib/Awt2dLibraries.gmk:468: recipe for target '/home/runner/work/android-openjdk-build-multiarch/android-openjdk-build-multiarch/openjdk/build/linux-aarch64-normal-server-release/jdk/objs/libawt/GraphicsPrimitiveMgr.o' failed
make[2]: *** Waiting for unfinished jobs....
BuildJdk.gmk:70: recipe for target 'libs-only' failed
make[1]: *** [libs-only] Error 2
make: *** [jdk-only] Error 2
/home/runner/work/android-openjdk-build-multiarch/android-openjdk-build-multiarch/openjdk//make/Main.gmk:116: recipe for target 'jdk-only' failed

At

for (pHdr = pStart; pHdr < pEnd; pHdr = PtrAddBytes(pHdr, size)) {

If I comment out the check pHdr < pEnd, then it built. But I want it to complete headless AWT to get Caciocavallo project working or wrap Graphics2D to Android Canvas.

include hotspot.agent, management and management.agent

johanvos · 2020-11-02T15:07:39Z

Excuse me, but why don't you port jdk8 instead?

This repository is auto-synced from upstream openjdk/jdk which is 16-ea currently. I don't see a reason why we want Java 8 on Android/ios while we have 15 and beyond on desktop?

khanhduytran0 · 2020-11-02T22:34:33Z

Simple reason for porting Java 8 is LTS available, but Java 11 have LTS too.

magicus · 2020-11-04T19:20:55Z

make/modules/java.desktop/lib/Awt2dLibraries.gmk

+ifeq ($(ENABLE_HEADLESS_ONLY), true)
+  LIBAWT_CFLAGS += -DHEADLESS
+endif
+ifeq ($(call isTargetOs, ios), android)


This looks wrong. Maybe you meant ifeq ($(call isTargetOs, android), true)?

openjdk · 2023-02-17T18:32:22Z

@johanvos this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout 8255240-excludemodules
git fetch https://git.openjdk.org/mobile master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

bridgekeeper · 2023-12-05T03:04:34Z

@johanvos This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper · 2024-01-02T03:57:48Z

@johanvos This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

thread #2, stop reason = breakpoint 1.1 * frame #0: 0x0000000100808a64 MobileOctober`failInApp at javalauncher.c:33:11 frame #1: 0x000000010093d24c MobileOctober`vm_notify_during_shutdown(error=<unavailable>, message=<unavailable>) at java.cpp:654:5 [opt] [artificial] frame #2: 0x000000010093d2d0 MobileOctober`vm_exit_during_initialization(ex=0x00000001015d9c10, message=<unavailable>) at java.cpp:689:3 [opt] frame #3: 0x000000010090b8ac MobileOctober`Exceptions::special_exception(thread=<unavailable>, file=<unavailable>, line=<unavailable>, h_exception=<unavailable>, h_name=0x00000001015d9c10, message="java/lang/Object") at exceptions.cpp:101:7 [opt] frame openjdk#4: 0x000000010090bcc8 MobileOctober`Exceptions::_throw_msg(thread=0x0000000102015400, file="src/hotspot/share/classfile/systemDictionary.cpp", line=325, name=0x00000001015d9c10, message="java/lang/Object", h_loader=Handle @ x24, h_protection_domain=Handle @ x22) at exceptions.cpp:193:7 [opt] frame openjdk#5: 0x0000000100a5e3a0 MobileOctober`handle_resolution_exception(class_name=0x00000001015d8100, throw_error=<unavailable>, __the_thread__=0x0000000102015400) at systemDictionary.cpp:0 [opt] frame openjdk#6: 0x0000000100a5e0c8 MobileOctober`SystemDictionary::resolve_or_fail(class_name=0x00000001015d8100, class_loader=<unavailable>, protection_domain=<unavailable>, throw_error=true, __the_thread__=0x0000000102015400) at systemDictionary.cpp:338:5 [opt] frame openjdk#7: 0x0000000100ab5aac MobileOctober`vmClasses::resolve(vmClassID, JavaThread*) [inlined] SystemDictionary::resolve_or_fail(class_name=<unavailable>, throw_error=true, __the_thread__=0x0000000102015400) at systemDictionary.hpp:94:12 [opt] frame openjdk#8: 0x0000000100ab5a98 MobileOctober`vmClasses::resolve(id=Object_klass_knum, __the_thread__=0x0000000102015400) at vmClasses.cpp:95:16 [opt] frame openjdk#9: 0x0000000100ab5afc MobileOctober`vmClasses::resolve_all(JavaThread*) at vmClasses.cpp:104:5 [opt] frame openjdk#10: 0x0000000100ab5af0 MobileOctober`vmClasses::resolve_all(JavaThread*) [inlined] vmClasses::resolve_through(last_id=Object_klass_knum, start_id=<unavailable>, __the_thread__=0x0000000102015400) at vmClasses.hpp:63:5 [opt] frame openjdk#11: 0x0000000100ab5af0 MobileOctober`vmClasses::resolve_all(__the_thread__=0x0000000102015400) at vmClasses.cpp:121:3 [opt] frame openjdk#12: 0x0000000100a607cc MobileOctober`SystemDictionary::initialize(__the_thread__=0x0000000102015400) at systemDictionary.cpp:1642:3 [opt] frame openjdk#13: 0x0000000100a954b4 MobileOctober`Universe::genesis(__the_thread__=0x0000000102015400) at universe.cpp:433:5 [opt] frame openjdk#14: 0x0000000100a971f0 MobileOctober`universe2_init() at universe.cpp:1068:3 [opt] frame openjdk#15: 0x0000000100920914 MobileOctober`init_globals2() at init.cpp:162:3 [opt] frame openjdk#16: 0x0000000100a921a0 MobileOctober`Threads::create_vm(args=<unavailable>, canTryAgain=0x000000016f7feecf) at threads.cpp:574:12 [opt] frame openjdk#17: 0x0000000100965cb8 MobileOctober`::JNI_CreateJavaVM(JavaVM **, void **, void *) [inlined] JNI_CreateJavaVM_inner(vm=<unavailable>, penv=<unavailable>, args=<unavailable>) at jni.cpp:3596:12 [opt] frame openjdk#18: 0x0000000100965c6c MobileOctober`JNI_CreateJavaVM(vm=0x000000016f7fef40, penv=0x000000016f7fef38, args=<unavailable>) at jni.cpp:3687:14 [opt] frame openjdk#19: 0x0000000100ae2aa0 MobileOctober`JavaMain + 256 frame openjdk#20: 0x0000000100ae4c98 MobileOctober`ThreadJavaMain + 12 frame openjdk#21: 0x00000001dca02338 libsystem_pthread.dylib`_pthread_start + 116

exclude some modules that are not relevant on Mobile.

6d5619b

Fix for JDK-8255240

openjdk bot added the rfr label Oct 22, 2020

johanvos requested a review from bobvandette October 22, 2020 13:01

Don't exclude java.desktop and jdk.accessibility modules.

83200b4

Don't build the native sound/window libs though. Add changes for generating char files and Xevent files (android)

fix fi/endif issue

f9040c9

bobvandette reviewed Oct 24, 2020

View reviewed changes

johanvos mentioned this pull request Oct 24, 2020

8254691: Enable GitHub actions for jfx repo openjdk/jfx#338

Closed

3 tasks

magicus suggested changes Oct 26, 2020

View reviewed changes

Fix OS/CPU mistake to not include SA on mobile

9130218

include hotspot.agent, management and management.agent

Allow to build Awt 2d libs for ios/android

1d51641

magicus reviewed Nov 4, 2020

View reviewed changes

openjdk bot added merge-conflict and removed rfr labels Feb 17, 2023

bridgekeeper bot closed this Jan 2, 2024

8255240: Mobile builds need to exclude some modules #10

8255240: Mobile builds need to exclude some modules #10

Uh oh!

Conversation

johanvos commented Oct 22, 2020 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewing

Webrev

Uh oh!

bridgekeeper bot commented Oct 22, 2020

Uh oh!

mlbridge bot commented Oct 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

bobvandette commented Oct 22, 2020

Uh oh!

johanvos commented Oct 22, 2020

Uh oh!

johanvos commented Oct 23, 2020

Uh oh!

johanvos commented Oct 24, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

khanhduytran0 commented Nov 1, 2020 • edited by bridgekeeper bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johanvos commented Oct 22, 2020 •

edited by openjdk bot

Loading

mlbridge bot commented Oct 22, 2020 •

edited

Loading

khanhduytran0 commented Nov 1, 2020 •

edited by bridgekeeper bot

Loading