-
Notifications
You must be signed in to change notification settings - Fork 3k
Compute bean raw types in a Set/HashSet before materializing them #51476
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
Conversation
It allows to avoid performing very badly when we have lots of beans of the same type, for instance when you have a lot of Panache repositories for various entities. We materialize things with Lists in the end, and they might be a bit more optimized than before for small non-singleton lists. The downside is that we have a bit more allocations but from my testing, it wasn't affecting startup times. Fixes quarkusio#51468
independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/ArcContainerImpl.java
Show resolved
Hide resolved
Status for workflow
|
| Status | Name | Step | Failures | Logs | Raw logs | Build scan |
|---|---|---|---|---|---|---|
| ❌ | Native Tests - Windows support | Setup GraalVM |
Logs | Raw logs | 🚧 |
You can consult the Develocity build scans.
Flaky tests - Develocity
⚙️ JVM Tests - JDK 21
📦 extensions/quartz/deployment
❌ io.quarkus.quartz.test.timezone.TriggerPrevFireTimeZoneTest.testScheduledJobs - History
expected: <2025-12-09T16:00:35Z> but was: <2025-12-09T16:00:36Z>-org.opentest4j.AssertionFailedError
org.opentest4j.AssertionFailedError: expected: <2025-12-09T16:00:35Z> but was: <2025-12-09T16:00:36Z>
at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1145)
at io.quarkus.quartz.test.timezone.TriggerPrevFireTimeZoneTest.testScheduledJobs(TriggerPrevFireTimeZoneTest.java:71)
⚙️ JVM Tests - JDK 21 Semeru
📦 extensions/micrometer-opentelemetry/deployment
❌ io.quarkus.micrometer.opentelemetry.deployment.compatibility.MicrometerTimedInterceptorTest.testTimeMethod_Uni - History
Stream has no elements-java.lang.IllegalArgumentException
java.lang.IllegalArgumentException: Stream has no elements
at io.quarkus.micrometer.opentelemetry.deployment.common.MetricDataFilter.lambda$lastReading$2(MetricDataFilter.java:213)
at java.base/java.util.Optional.orElseThrow(Optional.java:403)
at io.quarkus.micrometer.opentelemetry.deployment.common.MetricDataFilter.lastReading(MetricDataFilter.java:213)
at io.quarkus.micrometer.opentelemetry.deployment.common.MetricDataFilter.lastReadingDataPoint(MetricDataFilter.java:231)
at io.quarkus.micrometer.opentelemetry.deployment.compatibility.MicrometerTimedInterceptorTest.testTimeMethod_Uni(MicrometerTimedInterceptorTest.java:174)
at java.base/java.lang.reflect.Method.invoke(Method.java:586)
at io.quarkus.test.QuarkusUnitTest.runExtensionMethod(QuarkusUnitTest.java:532)
⚙️ MicroProfile TCKs Tests
📦 tcks/microprofile-lra
❌ org.eclipse.microprofile.lra.tck.TckRecoveryTests.testCancelWhenParticipantIsUnavailable - History
Expecting the metric Compensated callback was called Expected: a value equal to or greater than <1> but: <0> was less than <1>-java.lang.AssertionError
java.lang.AssertionError:
Expecting the metric Compensated callback was called
Expected: a value equal to or greater than <1>
but: <0> was less than <1>
at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
at org.eclipse.microprofile.lra.tck.TckRecoveryTests.assertMetricCallbackCalled(TckRecoveryTests.java:210)
at org.eclipse.microprofile.lra.tck.TckRecoveryTests.testCancelWhenParticipantIsUnavailable(TckRecoveryTests.java:195)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
mkouba
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good. In terms of allocations, this could sometimes allocate more (new HashSet() + List.copyOf()) but I think that in most cases it will be fine.
|
Yeah I did some allocation profiling after this change and it did allocate a bit more, which is not surprising. It wasn't that bad though. |
It allows to avoid performing very badly when we have lots of beans of the same type, for instance when you have a lot of Panache repositories for various entities.
We materialize things with Lists in the end, and they might be a bit more optimized than before for small non-singleton lists.
The downside is that we have a bit more allocations but from my testing, it wasn't affecting startup times.
Fixes #51468
We went from (see elements on the left side):
to
The rest of the ArC container initialization is mostly class loading.