Skip to content

Conversation

@mkouba
Copy link
Contributor

@mkouba mkouba commented May 31, 2022

Follows up on #25835

@mkouba mkouba requested review from Ladicek, Sanne and manovotn May 31, 2022 08:50
@quarkus-bot quarkus-bot bot added the area/arc Issue related to ARC (dependency injection) label May 31, 2022
Copy link
Contributor

@Ladicek Ladicek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK, but I guess the use of implementation types (HashSet or ArrayList) instead of interfaces (Set or List) might have been intentional, to avoid interface method lookups in a performance sensitive piece of code (invokevirtual vs. invokeinterface).

Copy link
Contributor

@manovotn manovotn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for noticing this

@Sanne
Copy link
Member

Sanne commented May 31, 2022

Looks OK, but I guess the use of implementation types (HashSet or ArrayList) instead of interfaces (Set or List) might have been intentional, to avoid interface method lookups in a performance sensitive piece of code (invokevirtual vs. invokeinterface).

right that's indeed my "preferred style" :) But it shouldn't matter here, it's ok.

@mkouba mkouba added the triage/waiting-for-ci Ready to merge when CI successfully finishes label May 31, 2022
@gastaldi gastaldi merged commit 21042ba into quarkusio:main May 31, 2022
@quarkus-bot quarkus-bot bot added this to the 2.10 - main milestone May 31, 2022
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/arc Issue related to ARC (dependency injection)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants