Support AWT image resize via new AWT extension #20239

galderz · 2021-09-17T14:55:13Z

This PR replaces #20148.

It enables AWT use cases, such as image resizing, using a dedicated AWT extension.

Note: the extension was generated with the create-extension command of the quarkus maven plugin.

gastaldi · 2021-09-17T14:57:32Z

@galderz can you rebase with the latest main? There are some conflicts

gastaldi · 2021-09-17T14:58:40Z

Also it would be great if you could also squash your commits

galderz · 2021-09-17T15:08:30Z

Yeah yeah, on it already...

galderz · 2021-09-17T15:10:52Z

@gastaldi Done. Let's see what CI says

extensions/awt/runtime/pom.xml

extensions/awt/runtime/src/main/resources/META-INF/quarkus-extension.yaml

integration-tests/awt/src/test/java/io/quarkus/awt/it/AwtImageResizeTest.java

quarkus-bot · 2021-09-17T15:27:23Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `55fb9f8`

Status	Name	Step	Failures	Logs	Raw logs
✖	Initial JDK 11 Build	`Build`	Failures	Logs	Raw logs

Failures

⚙️ Initial JDK 11 Build #

- Failing: integration-tests/awt

📦 integration-tests/awt

✖ Failed to execute goal net.revelc.code.formatter:formatter-maven-plugin:2.16.0:validate (default) on project quarkus-awt-integration-tests: File '/home/runner/work/quarkus/quarkus/integration-tests/awt/src/test/java/io/quarkus/awt/it/AwtImageResizeTest.java' has not been previously formatted. Please format file and commit before running validation!

Sanne · 2021-09-17T16:24:20Z

extensions/awt/runtime/src/main/java/io/quarkus/awt/runtime/graal/AwtFeature.java

+import com.oracle.svm.core.annotate.AutomaticFeature;
+
+@AutomaticFeature
+public class AwtFeature implements Feature {


galderz · 2021-09-17T16:27:44Z

@gastaldi @geoand All comments addressed.

Sanne · 2021-09-17T16:28:24Z

I have a general question.. how do you expect users to know they need to add this extension?

Should we consider it a "technical extension" and have other extensions such as those doing image manipulations depend on it?

Or perhaps it belongs in all applications, and we should pull it in automatically for all users?

I'm inclined to think it shoud be pulled in for every user automatically, provided there is no overhead. I'd normally expect this to not have any overhead, but you seem to have registered some classes for reflective access so that might force some code inclusion which would otherwise be identified as dead code. Why the reflective registrations?

geoand · 2021-09-17T16:31:15Z

The test also needs to be added to the native test configuration. I don't remember exactly where it is (and I'm on a phone right now), but @gastaldi can give you the exact location

gastaldi · 2021-09-17T16:36:11Z

The test also needs to be added to the native test configuration. I don't remember exactly where it is (and I'm on a phone right now), but @gastaldi can give you the exact location

It's in https://github.com/quarkusio/quarkus/blob/main/.github/native-tests.json

quarkus-bot · 2021-09-17T16:46:22Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `5189327`

Status	Name	Step	Failures	Logs	Raw logs
✖	Initial JDK 11 Build	`Verify extension dependencies`	⚠️ Check →	Logs	Raw logs

galderz · 2021-09-20T05:54:17Z

I have a general question.. how do you expect users to know they need to add this extension?

Should we consider it a "technical extension" and have other extensions such as those doing image manipulations depend on it?

Or perhaps it belongs in all applications, and we should pull it in automatically for all users?

Is there such big proportion of Quarkus apps doing image manipulations? Unless there is, I'd keep this as something that people bring in when they need it.

Or are you seeing this as something that should work out of the box?

I'm inclined to think it shoud be pulled in for every user automatically, provided there is no overhead. I'd normally expect this to not have any overhead, but you seem to have registered some classes for reflective access so that might force some code inclusion which would otherwise be identified as dead code. Why the reflective registrations?

The reflection registration for sun.awt.X11.XToolkit are for the Toolkit.getDefaultToolkit() code. IIRC sun.awt.X11GraphicsEnvironment is for the GraphicsEnvironment initialization.

I would expect the JNI registrations to have some cost too, and there are more of them compared to reflection registrations. My gut feeling is that you'll need even more JNI registrations for other AWT use cases.

galderz · 2021-09-20T08:19:01Z

Actually @Sanne, I think might have a point:

The hibernate-orm-panache native image build didn't complete, see extract maven logs:

"ForkJoinPool-2-worker-3" Id=20 in RUNNABLE
    at [email protected]/sun.java2d.loops.FillRect.<clinit>(FillRect.java:79)

What's happening is that as a result of removing Java2DSubstitutions, a load of JAXB graphics related code makes it to the universe, e.g.

│   │       │   │       │       │           │   │   │   │       │           │   │   │   │   │   │   │   │   │   │           │   ├── directly calls com.sun.xml.bind.v2.model.impl.RuntimeBuiltinLeafInfoImpl$10$1.<init>(com.sun.xml.bind.v2.model.impl.RuntimeBuiltinLeafInfoImpl$10):void id=103127 @bci=21 
│   │       │   │       │       │           │   │   │   │       │           │   │   │   │   │   │   │   │   │   │           │   │   └── directly calls java.awt.Component.<init>():void id=104120 @bci=6

To solve the problem above, we'd need an extension that is applied to all that takes care of the runtime initialization of all those packages. This could also be done directly in core, by moving io.quarkus.awt.runtime.graal.AwtFeature to core.

You could still have an optional extension for those who really want to use awt with reflection+jni registrations (or any optimised versions of those).

Thoughts?

galderz · 2021-09-22T12:52:39Z

Pushed a couple of commits to fix the tests that also failed in previous PR. They've fixed in different way this time:

ImageIOTestCase has been moved to integration tests for awt.
Tika extension now depends on awt extension. It also adds a feature to mark which packages need to be runtime initialized.

The only thing remaining then would be documentation. Where would it go?

quarkus-bot · 2021-09-22T13:13:24Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `1848da0`

Status	Name	Step	Failures	Logs	Raw logs
✖	Initial JDK 11 Build	`Build`	Failures	Logs	Raw logs

Failures

⚙️ Initial JDK 11 Build #

- Failing: extensions/tika/runtime integration-tests/awt 
! Skipped: devtools/bom-descriptor-json docs extensions/tika/deployment and 3 more

📦 extensions/tika/runtime

✖ Failed to execute goal net.revelc.code:impsort-maven-plugin:1.6.2:check (check-imports) on project quarkus-tika: Imports are not sorted in /home/runner/work/quarkus/quarkus/extensions/tika/runtime/src/main/java/io/quarkus/tika/runtime/graal/TikaFeature.java

📦 integration-tests/awt

✖ Failed to execute goal net.revelc.code:impsort-maven-plugin:1.6.2:check (check-imports) on project quarkus-awt-integration-tests: Imports are not sorted in /home/runner/work/quarkus/quarkus/integration-tests/awt/src/test/java/io/quarkus/awt/it/ImageIOTestCase.java

geoand · 2021-09-22T13:23:57Z

When everything is done, please squash the formatting commits

Sanne · 2021-09-22T13:44:58Z

I like the shape this is taking - no idea where to put docs though, I suppose that can be deferred as a separate issue as really there isn't much to document, we just need figure out how to make sure people know about this extension when hitting problems with awt.

geoand · 2021-09-22T13:47:18Z

we just need figure out how to make sure people know about this extension when hitting problems with awt

Hopefully discoverability via the tooling would be enough

galderz · 2021-09-22T14:00:14Z

When everything is done, please squash the formatting commits

Yeah sure. Just waiting for a final CI run to make sure I've not forgotten anything.

quarkus-bot · 2021-09-22T17:30:59Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `012d2a8`

Status	Name	Step	Failures	Logs	Raw logs
✖	MicroProfile TCKs Tests	`Verify`	Failures	Logs	Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ MicroProfile TCKs Tests #

- Failing: tcks/resteasy-reactive

📦 tcks/resteasy-reactive

✖ Failed to execute goal org.codehaus.mojo:exec-maven-plugin:3.0.0:exec (test) on project quarkus-tck-resteasy-reactive: Command execution failed.

📦 tcks/resteasy-reactive/target/testsuite/tests

✖ com.sun.ts.tests.jaxrs.platform.container.completioncallback.JAXRSClient0164.argumentIsNullWhenRegistredClassTest line 209 - More details - Source on GitHub

com.sun.ts.tests.jaxrs.common.JAXRSCommonClient$Fault: Unexpected response content No name has been set yet expecting NULL
	at com.sun.ts.tests.jaxrs.platform.container.completioncallback.JAXRSClient0164.assertString(JAXRSClient0164.java:458)
	at com.sun.ts.tests.jaxrs.platform.container.completioncallback.JAXRSClient0164.argumentIsNullWhenRegistredClassTest(JAXRSClient0164.java:209)

* Even though reflection and JNI registrations happen in the AWT extenssion, runtime initialization packages are defined in core. * This is done to avoid sun.java2d, and related classes, to be build time initialized, which causes issues with other modules. * Move ImageIO test from main to awt. * Make Apache Tika depend on AWT extension * Mark Tika packages that need runtime initialization for AWT. This definition can happen directly at the Tika extension level. Fix formatting

galderz · 2021-09-23T11:05:21Z

Squashed and pushed. This is ready to go in now.

geoand

Thanks!

quarkus-bot · 2021-09-23T15:32:32Z

Failing Jobs - Building `b6038fc`

Status	Name	Step	Failures	Logs	Raw logs
✖	Gradle Tests - JDK 11 Windows	`Build`	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
⌛	JVM Tests - JDK 16	`Build`	⚠️ Check →	Logs	Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ Gradle Tests - JDK 11 Windows #

- Failing: integration-tests/gradle

📦 integration-tests/gradle

✖ io.quarkus.gradle.devmode.MultiModuleIncludedBuildTest.main line 24 - More details - Source on GitHub

java.lang.AssertionError: 

Expecting actual:

geoand · 2021-09-23T15:36:59Z

@Sanne I'll leave the final decision up to you since you've been following this closely

Sanne · 2021-09-23T15:44:43Z

@Sanne I'll leave the final decision up to you since you've been following this closely

damn :)

thanks all! merged it

geoand · 2021-09-23T15:55:00Z

One more question: Should we backport this into 2.3? It seems harmless, no?

galderz · 2021-09-23T17:10:13Z

@geoand Yeah, the impact on existing code is minimal (just add awt/imageio/java2d packages for runtime init). For the rest of the code to kick in, an extension has to depend on awt specifically.

geoand · 2021-09-24T08:58:36Z

I am removing the backport label as a concequence of #11563

galderz · 2021-09-30T16:15:49Z

@geoand Can someone create an issue for #11563 and assign it to me? I've been digging into graalvm/mandrel#292

geoand · 2021-10-01T04:54:19Z

Done: #20491

galderz requested review from Sanne, maxandersen, gastaldi and geoand September 17, 2021 14:55

quarkus-bot bot added area/core area/dependencies Pull requests that update a dependency file labels Sep 17, 2021

gastaldi mentioned this pull request Sep 17, 2021

Draft support for AWT image resizing #20148

Closed

gastaldi linked an issue Sep 17, 2021 that may be closed by this pull request

AWT Graphics does not seem to work in native image #19789

Closed

galderz force-pushed the t_awt_image_resize_v4 branch from 2b5d57d to 55fb9f8 Compare September 17, 2021 15:10

gastaldi reviewed Sep 17, 2021

View reviewed changes

extensions/awt/runtime/pom.xml Show resolved Hide resolved

gastaldi reviewed Sep 17, 2021

View reviewed changes

extensions/awt/runtime/src/main/resources/META-INF/quarkus-extension.yaml Outdated Show resolved Hide resolved

geoand reviewed Sep 17, 2021

View reviewed changes

integration-tests/awt/src/test/java/io/quarkus/awt/it/AwtImageResizeTest.java Outdated Show resolved Hide resolved

Sanne reviewed Sep 17, 2021

View reviewed changes

galderz force-pushed the t_awt_image_resize_v4 branch from 55fb9f8 to 5189327 Compare September 17, 2021 16:27

galderz force-pushed the t_awt_image_resize_v4 branch from 5189327 to 5f2db3b Compare September 20, 2021 05:33

quarkus-bot bot added area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure labels Sep 20, 2021

galderz force-pushed the t_awt_image_resize_v4 branch from 4eb55df to a638a0f Compare September 22, 2021 08:36

quarkus-bot bot added the area/tika label Sep 22, 2021

galderz force-pushed the t_awt_image_resize_v4 branch from 012d2a8 to b6038fc Compare September 23, 2021 11:05

geoand approved these changes Sep 23, 2021

View reviewed changes

Sanne approved these changes Sep 23, 2021

View reviewed changes

Sanne added the release/noteworthy-feature label Sep 23, 2021

Sanne merged commit 103590b into quarkusio:main Sep 23, 2021

quarkus-bot bot added this to the 2.4 - main milestone Sep 23, 2021

geoand added the triage/backport? label Sep 23, 2021

zakkak mentioned this pull request Sep 24, 2021

Quarkus Tika IT fails with Mandrel 21.2 and 20.3 while passing with GraalVM CE 21.2 graalvm/mandrel#292

Closed

geoand removed the triage/backport? label Sep 24, 2021

galderz mentioned this pull request Oct 1, 2021

OptaPlanner AWT DataFlavor build time init error #20491

Closed

Karm mentioned this pull request Oct 22, 2021

AWT Extension, Decoders, Encoders, Geometry and Fonts #20850

Merged

Support AWT image resize via new AWT extension #20239

Support AWT image resize via new AWT extension #20239

Conversation

galderz commented Sep 17, 2021

gastaldi commented Sep 17, 2021

gastaldi commented Sep 17, 2021

galderz commented Sep 17, 2021

galderz commented Sep 17, 2021

quarkus-bot bot commented Sep 17, 2021 • edited Loading

Failing Jobs - Building 55fb9f8

Failures

⚙️ Initial JDK 11 Build #

📦 integration-tests/awt

Sanne Sep 17, 2021

Choose a reason for hiding this comment

galderz commented Sep 17, 2021

Sanne commented Sep 17, 2021

geoand commented Sep 17, 2021

gastaldi commented Sep 17, 2021

quarkus-bot bot commented Sep 17, 2021 • edited Loading

Failing Jobs - Building 5189327

galderz commented Sep 20, 2021

galderz commented Sep 20, 2021

galderz commented Sep 22, 2021

quarkus-bot bot commented Sep 22, 2021 • edited Loading

Failing Jobs - Building 1848da0

Failures

⚙️ Initial JDK 11 Build #

📦 extensions/tika/runtime

📦 integration-tests/awt

geoand commented Sep 22, 2021

Sanne commented Sep 22, 2021

geoand commented Sep 22, 2021

galderz commented Sep 22, 2021

quarkus-bot bot commented Sep 22, 2021 • edited Loading

Failing Jobs - Building 012d2a8

Failures

⚙️ MicroProfile TCKs Tests #

📦 tcks/resteasy-reactive

📦 tcks/resteasy-reactive/target/testsuite/tests

galderz commented Sep 23, 2021

geoand left a comment

Choose a reason for hiding this comment

quarkus-bot bot commented Sep 23, 2021

Failing Jobs - Building b6038fc

Failures

⚙️ Gradle Tests - JDK 11 Windows #

📦 integration-tests/gradle

geoand commented Sep 23, 2021

Sanne commented Sep 23, 2021

geoand commented Sep 23, 2021 • edited Loading

galderz commented Sep 23, 2021

geoand commented Sep 24, 2021

galderz commented Sep 30, 2021

geoand commented Oct 1, 2021

quarkus-bot bot commented Sep 17, 2021 •

edited

Loading

Failing Jobs - Building `55fb9f8`

quarkus-bot bot commented Sep 17, 2021 •

edited

Loading

Failing Jobs - Building `5189327`

quarkus-bot bot commented Sep 22, 2021 •

edited

Loading

Failing Jobs - Building `1848da0`

quarkus-bot bot commented Sep 22, 2021 •

edited

Loading

Failing Jobs - Building `012d2a8`

Failing Jobs - Building `b6038fc`

geoand commented Sep 23, 2021 •

edited

Loading