Speedup Hibernate ORM's enhancement of large models

[Sanne]

This works far better, in my local tests.

I feel it's ready, but it will require a fix on the ORM side to be merged first:

• HHH-18011 Parallel entity enhancement causes a classloader cache stampede hibernate/hibernate-orm#8263

This brings enhancement times of the large model provided as benchmark down to ~1 second, without enabling Quarkus's caching capabilities of caching enhancement of unmodified classes (which we should also enable, but it's a separate aspect)

[quarkus-bot]

/cc @gsmet (hibernate-orm), @yrodiere (hibernate-orm)

[yrodiere]

Thanks!

Seems safe to me, and it's great if it speeds things up so much, but could you please add a few comments? I'm afraid it's not as obvious for us as it is for you :)

[Sanne]

Updated to add the suggested comments; also now registering the Panache super types as "core types" so they'd also benefit from caching, if used.

[Sanne]

also now registering the Panache super types as "core types" so they'd also benefit from caching, if used.

Note to self: this implies that if we have live-reload tests in Panache which have entities using the same package name as Panache itself they might fail now for the silly reason of us skipping the work on such classes.

[Sanne]

We decided to use:
"jakarta.persistence.", "java.", "org.hibernate.annotations.", "io.quarkus.hibernate.reactive.panache.", "io.quarkus.hibernate.orm.panache.", "org.hibernate.search.mapper.pojo.mapping.definition.annotation.", "jakarta.validation.constraints."

TODO: check this doesn't break Panache integration tests as they might be running under the same package name.

[Sanne]

Rebased: simplified implementation taking advantage of eb69d59

[Sanne]

Ok this is ready now, it was waiting for ORM 6.5.1.

[yrodiere]

Thanks. LGTM, just a small doubt that there might be an obsolete comment.

[Sanne]

Please don't merge - I've pushed an experiment and need CI feedback.

[Sanne]

@yrodiere I understand what's going on now, unfortunatley sharing the ByteBuddy caches would require much more work - and is tricky.

So I suggest we merge this version: it shares the caches of "core types" (which we know are not being modified) but isolates the caches used for bytecode enhancement in threadlocals to ensure independent interactions. It also shared the Enhancer implementation across runs, and makes its initialization lazy.

It's taking the safer approach - there's room for improvement as this forces each thread to repeat quite some work, and also allocate large structures w/o sharing those, but that would require much more work to share them safely, so I'd rather take the 80% improvement now and keep it safe until we have time to add more stricter integration tests.

Ok to merge? I'd hit the button as it's already approved, but since several changes were made...

[mapuci]

offtopic:

@Sanne could you please take a look at this issue? It is kindof related to what you are doing here :)
#40719
There is something wrong with enhanced classes or even their classloaders...(in quarkus 2 and 3, but not in wildfly).
Reproducer repo contains enhanced .class files, so you can take a look at them.

I tried to reproduce problem with this branch, there is still a bug.

[yrodiere]

I'm really not sure using threadlocals without ever clearing them is safe in dev mode, but I'll let you check that, and merge if you are confident.

[Sanne]

Pushed a rebase, as I plan to resume this work this weekend.

[gsmet]

Shouldn't we use JAVA_V17 now? (line just below)

I would create a separate PR to do it but it's going to be in your way so I will let you address it :).

[Sanne]

good point :)
Amended

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 16d1cd0.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

---

Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 integration-tests/grpc-hibernate

✖ com.example.grpc.hibernate.BlockingRawTest.shouldAdd - History

• Condition with Lambda expression in com.example.grpc.hibernate.BlockingRawTestBase was not fulfilled within 30 seconds. - org.awaitility.core.ConditionTimeoutException

org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in com.example.grpc.hibernate.BlockingRawTestBase was not fulfilled within 30 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
	at com.example.grpc.hibernate.BlockingRawTestBase.shouldAdd(BlockingRawTestBase.java:59)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)

[dashorst]

Nice work @Sanne , looking forward to trying this out in a next quarkus release!