[GraalVM for JDK 22] quarkus-integration-test-jpa-postgresql test fails with latest GraalVM master

[jerboaa]

Describe the bug

The native integration test tracking that XML factories from the JDK are not included are failing with latest GraalVM master builds and JDK 22+25 early access. The failure looks like this (and has been seen from around Nov 22 onwards):

Error:  Tests run: 2, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 6.835 s <<< FAILURE! -- in io.quarkus.it.jpa.postgresql.JPAFunctionalityInGraalITCase
Error:  io.quarkus.it.jpa.postgresql.JPAFunctionalityInGraalITCase.verifyJdkXmlParsersHavebeenEcludedFromNative -- Time elapsed: 0.190 s <<< FAILURE!
org.opentest4j.AssertionFailedError: Type 'com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl' was found in the report in target/quarkus-integration-test-jpa-postgresql-999-SNAPSHOT-native-image-source-jar/reports/used_classes_quarkus-integration-test-jpa-postgresql-999-SNAPSHOT-runner_20231204_161950.txt
	at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:38)
	at org.junit.jupiter.api.Assertions.fail(Assertions.java:134)
	at io.quarkus.test.junit.nativeimage.ClassInclusionReport.assertContainsNot(ClassInclusionReport.java:67)
	at io.quarkus.it.jpa.postgresql.JPAFunctionalityInGraalITCase.verifyJdkXmlParsersHavebeenEcludedFromNative(JPAFunctionalityInGraalITCase.java:31)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:816)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

See:
https://github.com/graalvm/mandrel/actions/runs/7087354809/job/19288795063#step:12:521

And indeed the image size increased from ~74MB (from a Nov 22 build) to ~87MB which is about the same size as the one including the XML factories.

We see the same failure on JDK 21-based GraalVM CE builds. See:
https://github.com/graalvm/mandrel/actions/runs/7087354812/job/19288620329#step:12:522

[quarkus-bot]

/cc @Karm (mandrel), @galderz (mandrel), @zakkak (mandrel)

[zakkak]

It looks like this is caused by #35113. Interestingly it doesn't seem to fail with Mandrel 23.1 (see https://github.com/quarkusio/quarkus/actions/runs/6959004824/job/18938761459#step:14:557).

Part of the size increase is also noticeable with Mandrel 23.1. In https://github.com/quarkusio/quarkus/actions/runs/6959004824/job/18938761459#step:14:415 (after the merge) the reported image size is 78.90MB in total while in https://github.com/quarkusio/quarkus/actions/runs/6957069306/job/18929685671#step:14:415 (before the merge) the reported size was 73.81MB in total.

cc @yrodiere @marko-bekhta

[yrodiere]

I'm guessing this is caused by the addition of Jackson as a dependency to that module: https://github.com/quarkusio/quarkus/pull/35113/files#diff-a7c108a7c682a73ce46773f1cb0016dd8b8e59b212a9b3222cc7e22431e6f958

That was necessary to test JSON formats, but indeed I can see how this could cause nasty side-effects when generating native images.

I think we should just move the tests related to JSON to a separate module... maybe jpa-postgresql-withjson?

@marko-bekhta Do you think you could have a look? Beware, I refactored all these tests just a few days ago :)

[marko-bekhta]

Hey, yes, sure. I'll see what I can do 😃. And yeah, I've seen the updates 😉

[galderz]

I think i know what the problem is and I suspect this might affect other situations out there:

The issue starts to happen when oracle/graal@1ed2a58 was integrated. The effect that it has is that looking up a constant has the side effect of making that constant part of the image heap as an image heap constant. I've pinged @cstancu about this and whether it can be improved/changed.

Why is this a problem in this case? Both before and after the commit, svm would resolve the constructor of com.fasterxml.jackson.databind.ext.DOMSerialize. This is happening because it parses this path:

Decoding java.lang.Class.getDeclaredConstructor(Class[])
	at java.lang.Class.getDeclaredConstructor(Class.java)
	at com.fasterxml.jackson.databind.util.ClassUtil.findConstructor(ClassUtil.java:576)
	at com.fasterxml.jackson.databind.util.ClassUtil.createInstance(ClassUtil.java:560)
	at com.fasterxml.jackson.databind.ext.OptionalHandlerFactory.instantiate(OptionalHandlerFactory.java:249)
	at com.fasterxml.jackson.databind.ext.OptionalHandlerFactory.instantiate(OptionalHandlerFactory.java:237)
	at com.fasterxml.jackson.databind.ext.OptionalHandlerFactory.findSerializer(OptionalHandlerFactory.java:135)

And through that it discovers the constructor. The constructor is found because of the changes in #35113.

So, because the constructor is found (due to #35113) AND because the constructor now becomes an image heap constant (due to oracle/graal@1ed2a58), an entry point in the universe is created for the constructor, represented as a factory method. This is exactly what we see in the call tree report:

├── entry com.oracle.svm.core.code.FactoryMethodHolder.DOMSerializer_constructor_ed8d8ed1c0ba7927c499a8c3ec42227bb4ec6043():com.fasterxml.jackson.databind.ext.DOMSerializer id=633 
│   └── directly calls com.fasterxml.jackson.databind.ext.DOMSerializer.<init>():void id=12495 @bci=1 

And once you have that entry point, all the XML baggage is added to the universe.

Changes to GraalVM aside, I don't see an obvious substitution that can be done here, other than maybe reimplement OptionalHandlerFactory.findSerializer() to adjust for what makes sense for Quarkus, but there are fair few other serializers impacted here, so would need careful testing. Jackson could do with some further modularization and using service lookup patterns to decide what to load and when.

[galderz]

The problematic part of the commit is this change. So, this issue affects reflection related constant lookups at the very minimum (e.g. constructors discovered as a result of class lookups), which now become image heap constants, but other constants have a similar treatment too.

[galderz]

I've just created oracle/graal#8019 to try to find a solution to the problem upstream. In the mean time I'm looking at workarounds to avoid the issue. A substitution that reimplements findConstructor that doesn't call getModifiers might do the trick temporarily. Codrut has hinted there might be something that can improved upstream.

[galderz]

A draft PR with a workaround can be found in #37732.

[jerboaa]

This should be fixed with oracle/graal#8041. I still need to verify this is now gone for good.

[jerboaa]

FYI: The latest nightly CI run of ours didn't include the fix yet (it built revision 986213c418674b5987fbb240705e020ad186a03e which didn't seem to include any of the fix commits). The next run should include it. Then we'll know more.

[galderz]

I've quickly run it locally and the latest oracle master fixes this.