Skip to content

Conversation

@mirogaudi
Copy link
Contributor

No description provided.

@mirogaudi mirogaudi requested a review from SteKoe July 31, 2025 11:10
@mirogaudi mirogaudi requested a review from a team as a code owner July 31, 2025 11:10
@mirogaudi mirogaudi requested a review from erikpetzold July 31, 2025 11:48
this.source = source;
this.metadata = new LinkedHashMap<>();
metadata.forEach(this.metadata::put);
for (Map.Entry<String, String> entry : metadata.entrySet()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should fix possible performance issues with metadata.forEach(this.metadata::put);

Copy link
Member

Choose a reason for hiding this comment

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

Can you explain why there is a performance issue with the forEach but not with for loop? It is calling the same put method and I don't expect many entries here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having metadata.forEach(this.metadata::put); IntelliJ will complain about possible performance issues and suggest you to use this.metadata.putAll(metadata). If used, this will make a result map immutable and will brake the logic, so for example HazelcastEventStoreTest will go in a never ending loop.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, so for performance, putAll is better than putting each one separately... but for loop is not better than forEach, it just hides it from IntelliJ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly, it prevents possible "small enhancement" braking the code.

Copy link
Contributor

@ulischulte ulischulte left a comment

Choose a reason for hiding this comment

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

LGTM!

@mirogaudi mirogaudi merged commit 33c52b3 into master Aug 4, 2025
1 check passed
@mirogaudi mirogaudi deleted the chore/minor-clean-ups-6 branch August 4, 2025 07:21
@SteKoe SteKoe added this to the 3.5.2 milestone Sep 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants