-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
AWT Extension, Decoders, Encoders, Geometry and Fonts #20850
Conversation
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 4e83049
Failures⚙️ Initial JDK 11 Build #- Failing: integration-tests/awt
📦 integration-tests/awt✖ |
4e83049
to
8b93680
Compare
If @rkennke would like to see what we did to AWT libs in Quarkus... |
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 8b93680
Full information is available in the Build summary check run. Failures⚙️ JVM Tests - JDK 11 #- Failing: integration-tests/tika
📦 integration-tests/tika✖ ⚙️ JVM Tests - JDK 11 Windows #- Failing: extensions/tika/deployment integration-tests/awt
! Skipped: docs integration-tests/main integration-tests/tika 📦 extensions/tika/deployment✖
✖
📦 integration-tests/awt✖
⚙️ JVM Tests - JDK 17 #- Failing: integration-tests/tika
📦 integration-tests/tika✖ |
Nice work!
If you want to actually throw an exception, what you have done is fine. |
@Karm There are these tika integration test failures. Does that ring any bell?
|
Lemme address the failures in the CI: Tika integration tests on LinuxTika extension needs much more work on exhaustively torturing the doc parsers with doc formats, charsets encodings, metadata, various pdfs with embedded fonts etc., plus fuzzing it with malformed data. The current failure is caused by hitting a code path not covered for runtime initialization. Not related to the AWT extension. Although Quarkus being headless by default now links libs without X windowing support....still I don't think that is the case.
Tika on WindowsTika on Windows is trying to load AWT extension and to a native-image build with it, correctly throwing:
AWT on Windows is not supported and cannot be expected to work with the currently released Graal to my best knowledge. Tika and JPEG2000Looking at
While this is a character encoding, not an image library problem, I think it's worth mentioning that jai-image-io should not be necessary to decode JPEG2000 images. This AWT test suite decodes JPEG2000 (.jp2) using JDK built-in decoder. |
@jerboaa See my comment. If the character encoding error is new, it is only because AWT extension allowed Tika to go deeper to hit it. We should address it either in the Tika extension itself or better create another native extension like AWT, this time for charsets, characters encoding and support... Then if you include both such extension and AWT extension you have the full power of charsets and fonts in native image... My point being, the AWT extension (while being actually an ImageIO and Java2D and Fonts extension) is already very beefy, comprising image processing, Java2D and fonts (TrueType). So I am not sure adding charsets decoders from sun.nio.cs.ext to it would be good. I would rather Tika have it or create a new extension for it. |
I'll let quarkus devs decide what the desired approach should be on this. |
If there is a vibe it should be added to the AWT extension, then I would broaden the coverage with a whole new suite for charsets and their representations. The only thing that AWT tests now in this area is de facto English ASCII text rendered as TrueType fonts plus a single unicode emoji character. But I can see how working with fonts might lead to pulling the charsets work in it... rabbit hole :) |
8b93680
to
6ce40a0
Compare
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 6ce40a0
Full information is available in the Build summary check run. Failures⚙️ JVM Tests - JDK 11 Windows #- Failing: extensions/tika/deployment integration-tests/awt
! Skipped: docs integration-tests/main integration-tests/tika 📦 extensions/tika/deployment✖
✖
📦 integration-tests/awt✖
|
All tests are passing, those failures are intentional ones where either AWT or Tika (depending on AWT) is trying to compile Native image on Windows:
Windows AWT/ImageIO is not yet properly supported in Graal. So these should be excluded from Windows for the time being. |
Ah ok. But could you disable them on Windows then? We can't have CI fail. |
6ce40a0
to
4651c57
Compare
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 4651c57
Failures⚙️ JVM Tests - JDK 11 #- Failing: extensions/hibernate-orm/runtime
! Skipped: docs extensions/hibernate-envers/deployment extensions/hibernate-envers/runtime and 101 more 📦 extensions/hibernate-orm/runtime✖ ⚙️ JVM Tests - JDK 11 Windows #- Failing: extensions/hibernate-orm/runtime integration-tests/awt integration-tests/tika
! Skipped: docs extensions/hibernate-envers/deployment extensions/hibernate-envers/runtime and 101 more 📦 extensions/hibernate-orm/runtime✖ 📦 integration-tests/awt✖ 📦 integration-tests/tika✖ ⚙️ JVM Tests - JDK 17 #- Failing: extensions/hibernate-orm/runtime
! Skipped: docs extensions/hibernate-envers/deployment extensions/hibernate-envers/runtime and 101 more 📦 extensions/hibernate-orm/runtime✖ |
The PR needs a rebase after #21092 went in. |
4651c57
to
f5154dd
Compare
Calling
Causes a failure in building native image:
I wonder whether the fact that there is Full context: https://gist.github.com/Karm/819fcc75a71418819b204d754667e382 |
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building f5154dd
Failures⚙️ JVM Tests - JDK 11 #- Failing: integration-tests/awt integration-tests/main integration-tests/tika
📦 integration-tests/awt✖ 📦 integration-tests/main✖ 📦 integration-tests/tika✖ ⚙️ JVM Tests - JDK 11 Windows #- Failing: integration-tests/awt integration-tests/main integration-tests/tika
📦 integration-tests/awt✖ 📦 integration-tests/main✖ 📦 integration-tests/tika✖ ⚙️ JVM Tests - JDK 17 #- Failing: integration-tests/awt integration-tests/main integration-tests/tika
📦 integration-tests/awt✖ 📦 integration-tests/main✖ 📦 integration-tests/tika✖ ⚙️ Native Tests - AWT, ImageIO and Java2D #- Failing: integration-tests/awt
📦 integration-tests/awt✖ ⚙️ Native Tests - Main #- Failing: integration-tests/main
📦 integration-tests/main✖ ⚙️ Native Tests - Misc2 #- Failing: integration-tests/tika
📦 integration-tests/tika✖ |
I wonder what could have been the last straw in those last two commits for the heap on Windows. I hope it's (not) just the CI. The test is somewhat intensive in what is does with images (regarding quantity), it might be possible that it's too much for the GitHub Windows host and that those JVM AWT tests eat resources that are never freed. I will try to measure the difference on my Windows VM. The GC log seems to be missing from the GitHub action run, if I am not looking at a wrong place:
I have started the following matrix on Windows 2019 VM to see what would be the difference in the gc log I hopefully obtain: Results
Systems: Windows 2019 VM, 8 vCPU, 16G RAM set "MAVEN_OPTS=-Xmx1500m -XX:MaxMetaspaceSize=1g -Xlog:gc*=debug:file=windows-java-11.txt"
mvnw install -Dquickly & mvnw verify -f integration-tests/pom.xml --fail-at-end --batch-mode -DfailIfNoTests=false %MODULES% |
2d64e47
to
b2854a4
Compare
I'm trying something: not reusing forks for the AWT tests. Let's see how it goes. If the move of some extensions to the Quarkiverse is approved, it might also help with this issue. |
#20850 (comment) looks exactly like #21761 (comment), so I don't think this PR has introduced this problem (might make it worse though, dunno). See also: https://github.com/quarkusio/quarkus/pulls?q=is%3Apr+%22Java+heap+space%22 |
@gsmet Thx for looking into it. I updated the spreadsheet with a row
and while both completed successfully without a single failure, and took about the same amount of time ( 01:24h and 1:29h ), the gc log for
|
@Karm let's keep it this way, in this case, I think the history has a lot of value. |
@Karm congrats for this nice work and thanks! |
There is a a lot of evidence that the parameter is set to `java.awt.headless=true` in Quarkus by default. * Quarkus Building native Images: https://quarkus.io/guides/building-native-image) see default value for `java.awt.headless` * quarkusio/quarkus#20565 * quarkusio/quarkus#20850
There is a a lot of evidence that the parameter is set to `java.awt.headless=true` in Quarkus by default. * Quarkus Building native Images: https://quarkus.io/guides/building-native-image) see default value for `java.awt.headless` * quarkusio/quarkus#20565 * quarkusio/quarkus#20850 Signed-off-by: Gabriel Mainberger <gabriel.mainberger@vshn.net>
There is a a lot of evidence that the parameter is set to `java.awt.headless=true` in Quarkus by default. * Quarkus Building native Images: https://quarkus.io/guides/building-native-image) see default value for `java.awt.headless` * quarkusio/quarkus#20565 * quarkusio/quarkus#20850 Signed-off-by: Gabriel Mainberger <gabriel.mainberger@vshn.net>
There is a a lot of evidence that the parameter is set to `java.awt.headless=true` in Quarkus by default. * Quarkus Building native Images: https://quarkus.io/guides/building-native-image) see default value for `java.awt.headless` * quarkusio/quarkus#20565 * quarkusio/quarkus#20850 Signed-off-by: Gabriel Mainberger <gabriel.mainberger@vshn.net>
There is a a lot of evidence that the parameter is set to `java.awt.headless=true` in Quarkus by default. * Quarkus Building native Images: https://quarkus.io/guides/building-native-image) see default value for `java.awt.headless` * quarkusio/quarkus#20565 * quarkusio/quarkus#20850 Signed-off-by: Gabriel Mainberger <gabriel.mainberger@vshn.net> Co-authored-by: Gabriel Mainberger <gabriel.mainberger@vshn.net>
Fixes #20565
Fixes #13567
Fixes #12972
Fixes #12393
Fixes #8605
Fixes #21729
Fixes #21726
The good
Test suite
The actual merit of this PR is the test suite that takes all encoders and decoders for all supported JDK formats for a spin, plus exercises a fair portion of headless-relevant basic Java2D operations, including fonts loading and rendering. There is a README.md in the integration-tests/awt/doc dir.
Classes for reflection and JNI access were added step by step, with JDK 11 sources opened, until the TS started to pass with flying colors, pun intended.
Works on my machine ™️
I tested the extension with Mandrel 21.3 and native-image built with JDK 11.0.13+7 (early access) and 22.0 (dev) Mandrel 22.0 (dev) JDK 17.0.x (head 92ff652).
The test suite requires JDK 11.0.13 based native-image as its minimal version due to JDK-8254024. The TS runs on Windows in HotSpot mode, but the extension terminates in Native mode as AWT in native-image is not yet ready for Windows. Enabling AWT with native-image on Windows goes beyond the scope of Quarkus into GraalVM/Mandrel codebase.
The TS runs well on Centos 8 and I also tried it in a Github action with latest Mandrel dev on Ubuntu:
I don't have a modern enough Mac handy, so any feedback from running natively on Mac is much appreciated (when there is JDK 11.0.13 based GraalVM on that platform and if AWT headless is supposed to work there...).
Quickstart
See Quickstart README.md.
PR: quarkusio/quarkus-quickstarts#961
The bad
The extension is inherently fragile with respect to any changes in JDK codebase. The test suite coves some selected code paths to enable users to do common image operations, but it is by no means exhaustive. We will be running the AT TS in CI with new JDK updates and monitor regressions.
Call for help
I would like to kindly ask for hints in these Quarkus build flow related topics, marked with TODO in the source:
Terminate before native-image build starts on an unsupported platform (Windows ATTOW). This is how I do it. Is it an idiomatic way to do such a thing with Quarkus?When the JDK version the native-image distribution was based on is not what is expected, I would like to print a warning before the native image build starts. That seems to be a chicken-egg problem to me, see the current implementation that prints the warning after it's built. Perhaps refactoring the GraalVM Version would be an option. It is certain that these issues with particular JDK compatibility will occur in future, so the work wouldn't be just to accommodate one warning.THX @galderz for kicking off the AWT extension.
FYI @jerboaa @zakkak @geoand