ArC micro optimizations #18567

mkouba · 2021-07-09T13:14:44Z

trim list of removed Beans
trim Resource reference providers
trim active Beans
do not use LinkedList
share qualifier annotation literals and Type instances in the
generated ComponentsProvider

Co-authored-by: Sanne Grinovero sanne@hibernate.org

mkouba · 2021-07-09T13:54:16Z

I also tried to do some inexact and naive tests with the hibernate-orm-quickstart where exactly 50 beans is removed by default:

Build the project with the relevant quarkus version
Run the app
Send an HTT GET to localhost:8080/fruits
Run the GC several times
Dump the heap and calculate the retained size of ArcContainerImpl#removedBeans

I tested the main branch, Sanne's PR and this PR:

Branch	Approx. retained size of `ArcContainerImpl#removedBeans`
main	16 KB
the other PR	11 KB
this PR	8 KB

mkouba · 2021-07-09T14:10:50Z

@Sanne It would be great if you could run your tests again and confirm/disprove my findings. FYI I cherry-picked 5 of your commits and only left out those that modified the RemovedBean and RemovedBeanImpl classes. Thanks!

geoand · 2021-07-09T15:03:54Z

independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/CollectionHelpers.java

-                return Collections.emptySet();
+                return Set.of();


I didn't know this was actually better

I had never seen this method :) Seems it's new in Java 9.
But also it's a constant in both cases apparently... I suppose you prefer it because of style Martin ?

It's not better, I just wanted to be consistent here... ;-)

geoand

LGTM

Sanne · 2021-07-09T16:07:26Z

independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/CollectionHelpers.java

+                return Set.of(set.iterator().next());
+            case 2:
+                Iterator<T> it = set.iterator();
+                return Set.of(it.next(), it.next());


hum, Set.of( ) produces an object of 24 bytes, while Collections.singleton 16 ,,. previous was better?

So I suppose you mean the java.util.ImmutableCollections.Set12 which is used for both a singleton and a doubleton.

Previously, we did not use a specialized set for a doubleton, which is a pity because in CDI doubletons are quite common.

I don't think it's reasonable to sacrifice the consistency for 8 bytes...

Sanne · 2021-07-09T16:13:23Z

independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/RemovedBeanImpl.java

        this.description = description;
        this.types = CollectionHelpers.toImmutableSmallSet(types);
-        this.qualifiers = qualifiers != null ? CollectionHelpers.toImmutableSmallSet(qualifiers)
-                : Qualifiers.DEFAULT_QUALIFIERS;
+        this.qualifiers = CollectionHelpers.toImmutableSmallSet(qualifiers);


I don't understand this, it looks lik you rather prefer to always allocate a new wrapping collection? I was trying to avoid those when not necessary.

Note really, the qualifiers set is always null for default qualifiers and CollectionHelpers.toImmutableSmallSet() returns null for null param.

Sanne · 2021-07-09T16:14:07Z

...ojects/arc/processor/src/main/java/io/quarkus/arc/processor/ComponentsProviderGenerator.java

@@ -387,16 +387,26 @@ void addComponentInternal(ObserverInfo observer) {
        private final ResultHandle removedBeansHandle;
        private final ClassOutput classOutput;
        private ResultHandle tccl;
+        // Shared annotation literals for an individual addRemovedBeansX() method
+        private final Map<AnnotationInstanceKey, ResultHandle> sharedQualifers;


Is that worth it?

I think it is. For example, because of SR Config almost every app will have ~ 30 removed beans with the @ConfigProperty qualifier, which in turn results in a new AnotationLiteral instance stored in the removedBeans list. This way, we can reduce the number of annotation literal instances to 2 (for the hibernate-orm-quickstart).

gsmet · 2021-07-12T08:21:33Z

I think we should merge this. We can still improve later on top of this if needed.

- trim list of removed Beans - trim Resource reference providers - trim active Beans - do not use LinkedList - share qualifier annotation literals and Type instances in the generated ComponentsProvider Co-authored-by: Sanne Grinovero <sanne@hibernate.org>

quarkus-bot · 2021-07-12T09:24:30Z

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

🚫 This workflow run has been cancelled.

Failing Jobs - Building `26460c5`

⚠️ Artifacts of the workflow run were not available thus the report misses some details.

Status	Name	Step	Test failures	Logs	Raw logs
⌛	Gradle Tests - JDK 11 Windows	`Build`	⚠️ Check →	Logs	Raw logs
✖	JVM Tests - JDK 11	`Build`	Test failures	Logs	Raw logs
✖	JVM Tests - JDK 11 Windows	`Build`	Test failures	Logs	Raw logs
✖	JVM Tests - JDK 16	`Build`	Test failures	Logs	Raw logs
⌛	MicroProfile TCKs Tests	`Verify`	⚠️ Check →	Logs	Raw logs
⌛	Native Tests - Cache	`Build`	⚠️ Check →	Logs	Raw logs
⌛	Native Tests - HTTP	`Build`	⚠️ Check →	Logs	Raw logs
⌛	Native Tests - Messaging1	`Build`	⚠️ Check →	Logs	Raw logs
⌛	Native Tests - Messaging2	`Build`	⚠️ Check →	Logs	Raw logs
⌛	Native Tests - Misc1	`Build`	⚠️ Check →	Logs	Raw logs
⌛	Native Tests - Misc2		⚠️ Check →	Logs	Raw logs
⌛	Native Tests - Misc3		⚠️ Check →	Logs	Raw logs
⌛	Native Tests - Misc4		⚠️ Check →	Logs	Raw logs
⌛	Native Tests - Security1	`Build`	⚠️ Check →	Logs	Raw logs
⌛	Native Tests - Security2	`Build`	⚠️ Check →	Logs	Raw logs
⌛	Native Tests - Security3	`Build`	⚠️ Check →	Logs	Raw logs
⌛	Native Tests - Spring		⚠️ Check →	Logs	Raw logs
⌛	Native Tests - Windows - hibernate-validator		⚠️ Check →	Logs	Raw logs
⌛	Native Tests - gRPC		⚠️ Check →	Logs	Raw logs

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 extensions/arc/deployment

✖ io.quarkus.arc.test.autoproduces.AutoProducerMethodDisabledTest. - More details - Source on GitHub

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/arc/deployment

✖ io.quarkus.arc.test.autoproduces.AutoProducerMethodDisabledTest. - More details - Source on GitHub

⚙️ JVM Tests - JDK 16 #

📦 extensions/arc/deployment

✖ io.quarkus.arc.test.autoproduces.AutoProducerMethodDisabledTest. - More details - Source on GitHub

mkouba · 2021-07-13T09:24:33Z

The PR is ready now...

quarkus-bot bot added the area/arc Issue related to ARC (dependency injection) label Jul 9, 2021

mkouba requested review from manovotn, Sanne, geoand and Ladicek July 9, 2021 14:11

geoand reviewed Jul 9, 2021

View reviewed changes

geoand approved these changes Jul 9, 2021

View reviewed changes

Sanne reviewed Jul 9, 2021

View reviewed changes

mkouba marked this pull request as ready for review July 12, 2021 08:24

ArC micro optimizations

ffa6e30

- trim list of removed Beans - trim Resource reference providers - trim active Beans - do not use LinkedList - share qualifier annotation literals and Type instances in the generated ComponentsProvider Co-authored-by: Sanne Grinovero <sanne@hibernate.org>

mkouba force-pushed the arc-removed-beans-opt branch from 26460c5 to ffa6e30 Compare July 12, 2021 09:22

gsmet merged commit 05ab2b6 into quarkusio:main Jul 13, 2021

quarkus-bot bot added this to the 2.1 - main milestone Jul 13, 2021

mkouba mentioned this pull request Jul 20, 2021

Avoid loading the list of ArC removed beans into the heap #18345

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ArC micro optimizations #18567

ArC micro optimizations #18567

mkouba commented Jul 9, 2021

mkouba commented Jul 9, 2021

mkouba commented Jul 9, 2021

geoand Jul 9, 2021

Sanne Jul 9, 2021

mkouba Jul 12, 2021

geoand left a comment

Sanne Jul 9, 2021

mkouba Jul 12, 2021

Sanne Jul 9, 2021

mkouba Jul 12, 2021

Sanne Jul 9, 2021

mkouba Jul 12, 2021

gsmet commented Jul 12, 2021

quarkus-bot bot commented Jul 12, 2021 •

edited

Loading

Failing Jobs - Building `26460c5`

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 extensions/arc/deployment

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/arc/deployment

⚙️ JVM Tests - JDK 16 #

📦 extensions/arc/deployment

mkouba commented Jul 13, 2021

ArC micro optimizations #18567

ArC micro optimizations #18567

Conversation

mkouba commented Jul 9, 2021

mkouba commented Jul 9, 2021

mkouba commented Jul 9, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geoand left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gsmet commented Jul 12, 2021

quarkus-bot bot commented Jul 12, 2021 • edited Loading

Failing Jobs - Building 26460c5

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 extensions/arc/deployment

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/arc/deployment

⚙️ JVM Tests - JDK 16 #

📦 extensions/arc/deployment

mkouba commented Jul 13, 2021

quarkus-bot bot commented Jul 12, 2021 •

edited

Loading

Failing Jobs - Building `26460c5`