native-image optimisations to allow excluding the JDK's XML parser from a PostgreSQL / Hibernate app #17902

Sanne · 2021-06-14T21:34:15Z

This fixes a regression in RSS memory consumption in Quarkus "crud" based examples.

It turns out the AutomaticFeature of GraalVM which triggers inclusion of the XML parsers is triggered by the combination of having an Hibernate ORM's PostgresUUIDType and the postgresql JDBC driver.

We include now an additional AutomaticFeature which helps the compiler to better identify this code path is unreacheable (when and only when it actually is). Comments in code provide more detail.

quarkus-bot · 2021-06-14T21:34:16Z

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

title should preferably start with an uppercase character (if it makes sense!)

This message is automatically generated by a bot.

yrodiere

Looks great! I'm just not sure that the tests leveraging the XML feature are enough, see below.
(Also a comment about logs, but feel free to ignore it)

...postgresql/runtime/src/main/java/io/quarkus/jdbc/postgresql/runtime/graal/SQLXLMFeature.java

...tgresql-withxml/src/main/java/io/quarkus/it/jpa/postgresql/JPAFunctionalityTestEndpoint.java

galderz · 2021-06-15T11:27:06Z

test-framework/junit5/src/main/java/io/quarkus/test/junit/nativeimage/ClassInclusionReport.java

+ * This is a general utility to assert via
+ * unit testing which classes have been included in a native-image.
+ */
+public final class ClassInclusionReport {


Having this might enable tracking noticeable class inclusion increases over different versions?

yes exactly! Both caused by changes in our code and changes in GraalVM - it will be tested on each build, we just need to use this test helper more frequently, and make sensible choices such as for which types we consider the contract broken.

Sanne · 2021-06-15T16:58:12Z

I've had another couple of challenges, but fixed now.

Also opened:

XMLParsersRegistration needs an additional resource bundle oracle/graal#3479

quarkus-bot · 2021-06-15T20:03:21Z

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

Failing Jobs - Building `01b0bcb`

Status	Name	Step	Test failures	Logs	Raw logs
✖	JVM Tests - JDK 11	`Build`	Test failures	Logs	Raw logs
✖	JVM Tests - JDK 16	`Build`	Test failures	Logs	Raw logs
✖	Native Tests - Data5	`Build`	Test failures	Logs	Raw logs

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 integration-tests/jpa-postgresql-withxml

✖ io.quarkus.it.jpa.postgresql.JPAFunctionalityTest.testJPAFunctionalityFromServlet - More details - Source on GitHub

⚙️ JVM Tests - JDK 16 #

📦 integration-tests/jpa-postgresql-withxml

✖ io.quarkus.it.jpa.postgresql.JPAFunctionalityTest.testJPAFunctionalityFromServlet - More details - Source on GitHub

⚙️ Native Tests - Data5 #

📦 integration-tests/jpa-postgresql-withxml

✖ io.quarkus.it.jpa.postgresql.JPAFunctionalityInGraalITCase.testJPAFunctionalityFromServlet - More details - Source on GitHub

quarkus-bot · 2021-06-15T23:47:05Z

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

Failing Jobs - Building `4c89272`

Status	Name	Step	Test failures	Logs	Raw logs
⌛	MicroProfile TCKs Tests	`Verify`	⚠️ Check →	Logs	Raw logs
✖	Native Tests - Data5	`Build`	Test failures	Logs	Raw logs

Full information is available in the Build summary check run.

Test Failures

⚙️ Native Tests - Data5 #

📦 integration-tests/jpa-postgresql-withxml

✖ io.quarkus.it.jpa.postgresql.JPAFunctionalityInGraalITCase.testJPAFunctionalityFromServlet - More details - Source on GitHub

yrodiere

Tests seem to be failing, unfortunately. I think I also spotted dead code.

...tgresql-withxml/src/main/java/io/quarkus/it/jpa/postgresql/JPAFunctionalityTestEndpoint.java

Sanne · 2021-06-16T09:13:35Z

The native test failure is puzzling me, it looks like it wasn't run with the latest version I pushed.

the logs specifically mention:

2021-06-15T20:47:28.3038956Z Quarkus's automatic feature for GraalVM native images: enabling support for XML processing in the PosgreSQL JDBC driver

which is triggering this hot-fix:

ResourcesRegistry resourcesRegistry = ImageSingletons.lookup(ResourcesRegistry.class);
resourcesRegistry.addResourceBundles("com.sun.org.apache.xalan.internal.xsltc.compiler.util.ErrorMessages");

Yet compilation fails with:

Caused by: com.oracle.svm.core.jdk.UnsupportedFeatureError: Resource bundle lookup must be loaded during native image generation: class com.sun.org.apache.xalan.internal.xsltc.compiler.util.ErrorMessages

Yesterday I've been re-verifying this need by removing those lines and then re-introducing them a couple of times, it looks like the build triggered with one of the intentionally failing cases. The failing log mentions git ref b13ed8f50c15c623607fedf9c5ee561b0ea1bd1a, unfortunately that reference was pruned so I can't double- check this theory. Let's try again..

yrodiere

LGTM, let's see if tests pass... :)

Sanne · 2021-06-16T11:48:48Z

Looks like the failure is triggered by the fact that the GraalVM version I'm using locally is slightly different, I'll investigate.

quarkus-bot · 2021-06-16T12:32:01Z

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

Failing Jobs - Building `d9af480`

Status	Name	Step	Test failures	Logs	Raw logs
✖	Native Tests - Data5	`Build`	Test failures	Logs	Raw logs

Full information is available in the Build summary check run.

Test Failures

⚙️ Native Tests - Data5 #

📦 integration-tests/jpa-postgresql-withxml

✖ io.quarkus.it.jpa.postgresql.JPAFunctionalityInGraalITCase.testJPAFunctionalityFromServlet - More details - Source on GitHub

Sanne · 2021-06-16T13:00:47Z

it's a problem with locales mismatching, making the resource bundle lookup fail. This works automagically on my local GraalVM build, but not yet on the tag we're using.

…sing capabilities only when required

…efault features

…-image

Sanne · 2021-06-16T15:40:54Z

@yrodiere technically I feel this was probably a different issue :) but solved it by adding yet another substitution UnsupportedTransformerFactoryFeatures. Hopefully CI will finally agree.

Sanne · 2021-06-16T15:44:13Z

part of the problem is that the matching resourcebundles don't even exist for some of the languages we're triggering. So the inclusion rule would only include the ones found (and even then not all), but the ones missing at runtime would cause a critical crash as GraalVM would prefer to indicate that they have likely not been registered for inclusion, rather than treating them as really not existent.
Funnily enough, this is triggered by the DefaultPGXmlFactoryFactory from postgresql as it attempts to specify attributes which are only valid on (I suppose?) much older JVM versions.

quarkus-bot · 2021-06-16T18:52:45Z

Failing Jobs - Building `962dc02`

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

Full information is available in the Build summary check run.

Test Failures

⚙️ MicroProfile TCKs Tests #

📦 tcks/microprofile-config

✖ org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testConfigPropertiesDefaultOnBean line 172 - More details - Source on GitHub

✖ org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testConfigPropertiesNoPrefixOnBean line 149 - More details - Source on GitHub

✖ org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testConfigPropertiesNoPrefixOnBeanThenSupplyPrefix line 156 - More details - Source on GitHub

✖ org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testConfigPropertiesPlainInjection line 106 - More details - Source on GitHub

✖ org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testConfigPropertiesWithPrefix line 115 - More details - Source on GitHub

✖ org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testConfigPropertiesWithoutPrefix line 132 - More details - Source on GitHub

✖ org.eclipse.microprofile.config.tck.ConfigPropertiesTest.testNoConfigPropertiesAnnotationInjection line 165 - More details - Source on GitHub

Sanne · 2021-06-16T19:42:05Z

ok so quarkus-tck-microprofile-context-propagation failed, but this module doesn't depend on these. I guess I'll finally merge - thanks @yrodiere !

Sanne added area/hibernate-orm Hibernate ORM area/native-image triage/backport? labels Jun 14, 2021

Sanne requested a review from yrodiere June 14, 2021 21:34

quarkus-bot bot added area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure area/persistence OBSOLETE, DO NOT USE area/testing labels Jun 14, 2021

yrodiere reviewed Jun 15, 2021

View reviewed changes

galderz reviewed Jun 15, 2021

View reviewed changes

Sanne force-pushed the XmlParsingOpt branch from 013cba0 to 01b0bcb Compare June 15, 2021 16:57

Sanne force-pushed the XmlParsingOpt branch from 01b0bcb to 4c89272 Compare June 15, 2021 20:19

yrodiere reviewed Jun 16, 2021

View reviewed changes

...tgresql-withxml/src/main/java/io/quarkus/it/jpa/postgresql/JPAFunctionalityTestEndpoint.java Outdated Show resolved Hide resolved

Sanne force-pushed the XmlParsingOpt branch from 4c89272 to d9af480 Compare June 16, 2021 09:02

yrodiere approved these changes Jun 16, 2021

View reviewed changes

yrodiere added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jun 16, 2021

Introduce substitutions for class org.postgresql.jdbc.PgSQLXML

0541c82

Sanne added 4 commits June 16, 2021 16:34

Introduce an AutomaticFeature for jdbc-postgresql enabling XML proces…

ec2752c

…sing capabilities only when required

Introduce a Substitution to fix PostgreSQL's driver handling of XML d…

92a05a2

…efault features

Integration test asserting which classes are being included in native…

d688bcf

…-image

Add new integration test module to test the native-image autofeature

962dc02

Sanne force-pushed the XmlParsingOpt branch from d9af480 to 962dc02 Compare June 16, 2021 15:39

yrodiere approved these changes Jun 16, 2021

View reviewed changes

Sanne merged commit f2b5115 into quarkusio:main Jun 16, 2021

quarkus-bot bot added this to the 2.1 - main milestone Jun 16, 2021

quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jun 16, 2021

Sanne deleted the XmlParsingOpt branch June 16, 2021 19:42

gsmet modified the milestones: 2.1 - main, 2.0.0.Final Jun 21, 2021

gsmet removed the triage/backport? label Jun 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

native-image optimisations to allow excluding the JDK's XML parser from a PostgreSQL / Hibernate app #17902

native-image optimisations to allow excluding the JDK's XML parser from a PostgreSQL / Hibernate app #17902

Sanne commented Jun 14, 2021

quarkus-bot bot commented Jun 14, 2021

yrodiere left a comment

galderz Jun 15, 2021

Sanne Jun 15, 2021 •

edited

Loading

Sanne commented Jun 15, 2021

quarkus-bot bot commented Jun 15, 2021 •

edited

Loading

Failing Jobs - Building `01b0bcb`

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 integration-tests/jpa-postgresql-withxml

⚙️ JVM Tests - JDK 16 #

📦 integration-tests/jpa-postgresql-withxml

⚙️ Native Tests - Data5 #

📦 integration-tests/jpa-postgresql-withxml

quarkus-bot bot commented Jun 15, 2021 •

edited

Loading

Failing Jobs - Building `4c89272`

Test Failures

⚙️ Native Tests - Data5 #

📦 integration-tests/jpa-postgresql-withxml

yrodiere left a comment

Sanne commented Jun 16, 2021 •

edited

Loading

yrodiere left a comment

Sanne commented Jun 16, 2021

quarkus-bot bot commented Jun 16, 2021 •

edited

Loading

Failing Jobs - Building `d9af480`

Test Failures

⚙️ Native Tests - Data5 #

📦 integration-tests/jpa-postgresql-withxml

Sanne commented Jun 16, 2021

Sanne commented Jun 16, 2021

Sanne commented Jun 16, 2021

quarkus-bot bot commented Jun 16, 2021

Sanne commented Jun 16, 2021

native-image optimisations to allow excluding the JDK's XML parser from a PostgreSQL / Hibernate app #17902

native-image optimisations to allow excluding the JDK's XML parser from a PostgreSQL / Hibernate app #17902

Conversation

Sanne commented Jun 14, 2021

quarkus-bot bot commented Jun 14, 2021

yrodiere left a comment

Choose a reason for hiding this comment

galderz Jun 15, 2021

Choose a reason for hiding this comment

Sanne Jun 15, 2021 • edited Loading

Choose a reason for hiding this comment

Sanne commented Jun 15, 2021

quarkus-bot bot commented Jun 15, 2021 • edited Loading

Failing Jobs - Building 01b0bcb

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 integration-tests/jpa-postgresql-withxml

⚙️ JVM Tests - JDK 16 #

📦 integration-tests/jpa-postgresql-withxml

⚙️ Native Tests - Data5 #

📦 integration-tests/jpa-postgresql-withxml

quarkus-bot bot commented Jun 15, 2021 • edited Loading

Failing Jobs - Building 4c89272

Test Failures

⚙️ Native Tests - Data5 #

📦 integration-tests/jpa-postgresql-withxml

yrodiere left a comment

Choose a reason for hiding this comment

Sanne commented Jun 16, 2021 • edited Loading

yrodiere left a comment

Choose a reason for hiding this comment

Sanne commented Jun 16, 2021

quarkus-bot bot commented Jun 16, 2021 • edited Loading

Failing Jobs - Building d9af480

Test Failures

⚙️ Native Tests - Data5 #

📦 integration-tests/jpa-postgresql-withxml

Sanne commented Jun 16, 2021

Sanne commented Jun 16, 2021

Sanne commented Jun 16, 2021

quarkus-bot bot commented Jun 16, 2021

Failing Jobs - Building 962dc02

Test Failures

⚙️ MicroProfile TCKs Tests #

📦 tcks/microprofile-config

Sanne commented Jun 16, 2021

Sanne Jun 15, 2021 •

edited

Loading

quarkus-bot bot commented Jun 15, 2021 •

edited

Loading

Failing Jobs - Building `01b0bcb`

quarkus-bot bot commented Jun 15, 2021 •

edited

Loading

Failing Jobs - Building `4c89272`

Sanne commented Jun 16, 2021 •

edited

Loading

quarkus-bot bot commented Jun 16, 2021 •

edited

Loading

Failing Jobs - Building `d9af480`

Failing Jobs - Building `962dc02`