Skip to content
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

GraalVM 24.0/23.1: JPAFunctionalityInGraalITCase.verifyJDKXMLParsersAreIncluded fails #35676

Closed
jerboaa opened this issue Sep 1, 2023 · 9 comments · Fixed by #35780
Closed
Assignees
Labels
area/mandrel kind/bug Something isn't working
Milestone

Comments

@jerboaa
Copy link
Contributor

jerboaa commented Sep 1, 2023

Describe the bug

With a latest Graal master (24.0, possibly also 23.1 build) the JPAFunctionalityInGraalITCase.verifyJDKXMLParsersAreIncluded integration test in native fails, no longer finding TransformerFactory in the report. I haven't looked into the details yet.

Error:  io.quarkus.it.jpa.postgresql.JPAFunctionalityInGraalITCase.verifyJDKXMLParsersAreIncluded -- Time elapsed: 0.179 s <<< FAILURE!
org.opentest4j.AssertionFailedError: Type 'javax.xml.transform.TransformerFactory' was not found in the report in target/quarkus-integration-test-jpa-postgresql-withxml-999-SNAPSHOT-native-image-source-jar/reports/used_classes_quarkus-integration-test-jpa-postgresql-withxml-999-SNAPSHOT-runner_20230901_021852.txt

See:
https://github.com/graalvm/mandrel/actions/runs/6044323984/job/16403307103#step:12:713

@jerboaa jerboaa added the kind/bug Something isn't working label Sep 1, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Sep 1, 2023

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

@jerboaa
Copy link
Contributor Author

jerboaa commented Sep 4, 2023

Reproduced locally. Seems like the instantiation fails with an error?

$ grep -n TransformerFactory reports/used_classes_quarkus-integration-test-jpa-postgresql-withxml-999-SNAPSHOT-runner_20230904_180113.txt
1165:com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl
6794:javax.xml.transform.TransformerFactoryConfigurationError

/cc @Sanne Any pointers perhaps?

@jerboaa
Copy link
Contributor Author

jerboaa commented Sep 4, 2023

Is this known to work with JDK 21?

@zakkak
Copy link
Contributor

zakkak commented Sep 6, 2023

The test passes in JVM mode with JDK 21.

Tested with:

./mvnw -pl integration-tests/jpa-postgresql-withxml -Dtest-containers -Dstart-containers -Dnative.surefire.skip -Dformat.skip -Dno-descriptor-tests clean verify

Note that the failing assertion is only for native-image runs though, so testing in JVM mode doesn't say much about it.

@jerboaa
Copy link
Contributor Author

jerboaa commented Sep 6, 2023

@zakkak Feel free to debug this on the native side. It looks like I won't be able to before Friday or so.

@zakkak zakkak self-assigned this Sep 6, 2023
@zakkak
Copy link
Contributor

zakkak commented Sep 6, 2023

The test fails here:

        //And finally verify we included the JDK XML by triggering
        //io.quarkus.jdbc.postgresql.runtime.graal.SQLXLMFeature
        report.assertContains(javax.xml.transform.TransformerFactory.class);

According to the logs SQLXLMFeature is indeed triggered:

Quarkus' automatic feature for GraalVM native images: enabling support for XML processing in the PostgreSQL JDBC driver

Looking for javax.xml.tranform in the used classes I see:

6771:javax.xml.transform.FactoryFinder
6772:javax.xml.transform.FactoryFinder$$Lambda$5a39e679f51666ce1c59d72f375f1ff0b1ecd490
6773:javax.xml.transform.FactoryFinder$$Lambda$5d8bd9c0dbbba726a06e35947adff460f55edd61
6774:javax.xml.transform.FactoryFinder$$Lambda$83d8a7fc5d68c5565f8d97170af17eddbdd633e1
6775:javax.xml.transform.FactoryFinder$$Lambda$f22c466d679458ecf16f7edd78c714b092bbb4e9
6776:javax.xml.transform.FactoryFinder$1
6777:javax.xml.transform.TransformerException
6778:javax.xml.transform.TransformerFactoryConfigurationError
6779:javax.xml.transform.dom.DOMResult
6780:javax.xml.transform.dom.DOMSource
6781:javax.xml.transform.sax.SAXResult
6782:javax.xml.transform.sax.SAXSource
6783:javax.xml.transform.stax.StAXResult
6784:javax.xml.transform.stream.StreamResult
6785:javax.xml.transform.stream.StreamSource

Which doesn't include javax.xml.transform.TransformerFactory as reported by the assertion.

Commenting out the failing assertion the test passes.

@Sanne
Copy link
Member

Sanne commented Sep 6, 2023

It's possible that my relatively old assertion is a bit fragile, the JDK might have made some improvements in this area
(I didn't check though, sorry I'm away).

Feel free to adapt the test: the purpose was primarily to detect the XML parser would be included in this scenario, as opposed to the other test which verifies that it gets excluded when we don't need it.

@zakkak
Copy link
Contributor

zakkak commented Sep 6, 2023

Interestingly, despite javax.xml.transform.TransformerFactory not appearing in the used classes report, when compiling with debug symbols generation enabled the said class is present in the debug info, as is its newInstance method. 👀

@zakkak
Copy link
Contributor

zakkak commented Sep 6, 2023

Apparently newInstance is pretty trivial and is getting inlined.

    public static TransformerFactory newInstance()
        throws TransformerFactoryConfigurationError {

        return FactoryFinder.find(
            /* The default property name according to the JAXP spec */
            TransformerFactory.class,
            /* The fallback implementation class name, XSLTC */
            "com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl");
    }

Passing -H:-InlineBeforeAnalysis makes the test pass again.

I suggest checking for com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl instead of javax.xml.transform.TransformerFactory. WDYT?

zakkak added a commit to zakkak/quarkus that referenced this issue Sep 6, 2023
javax.xml.transform.TransformerFactory does not appear as used in
GraalVM for JDK 21's reports due to inlining.

Closes quarkusio#35676
zakkak added a commit to zakkak/quarkus that referenced this issue Sep 6, 2023
javax.xml.transform.TransformerFactory does not appear as used in
GraalVM for JDK 21's reports due to inlining.

Closes quarkusio#35676
zakkak added a commit to zakkak/quarkus that referenced this issue Sep 7, 2023
javax.xml.transform.TransformerFactory does not appear as used in
GraalVM for JDK 21's reports due to inlining.

Closes quarkusio#35676
zakkak added a commit to zakkak/quarkus that referenced this issue Sep 7, 2023
javax.xml.transform.TransformerFactory does not appear as used in
GraalVM for JDK 21's reports due to inlining.

Closes quarkusio#35676
zakkak added a commit to zakkak/quarkus that referenced this issue Sep 7, 2023
javax.xml.transform.TransformerFactory does not appear as used in
GraalVM for JDK 21's reports due to inlining.

Closes quarkusio#35676
@quarkus-bot quarkus-bot bot added this to the 3.5 - main milestone Sep 11, 2023
holly-cummins pushed a commit to holly-cummins/quarkus that referenced this issue Feb 8, 2024
javax.xml.transform.TransformerFactory does not appear as used in
GraalVM for JDK 21's reports due to inlining.

Closes quarkusio#35676
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/mandrel kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants