Skip to content
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

Replace orElse with orElseGet to avoid calling too many times. #11542

Merged

Conversation

Technoboy-
Copy link
Contributor

@Technoboy- Technoboy- commented Aug 3, 2021

Motivation

Use orElseGet when orElse(xxx) is not a raw value, otherwise it will calling the value every time.

Documentation

This is an enhancement, no need to update doc.

@Technoboy- Technoboy- force-pushed the replace-orElse-with-orElseGet branch from 1c8f7d6 to 602ea02 Compare August 3, 2021 10:07
@Technoboy-
Copy link
Contributor Author

/pulsarbot run-failure-checks

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Lgtm

@eolivelli
Copy link
Contributor

The patch looks good.

I am not sure we achieve some visible improvement because in anyway you are allocating an instance of the lambda expression. Probably the overhead of instantiating such instance will be lower.

I am +1

@merlimat
Copy link
Contributor

merlimat commented Aug 3, 2021

I am not sure we achieve some visible improvement because in anyway you are allocating an instance of the lambda expression. Probably the overhead of instantiating such instance will be lower.

If the lambda is not capturing any reference, like TopicPolicies::new it will not need an allocation.

@eolivelli
Copy link
Contributor

I am not sure if it really happens.
I would be curious to verify if it really the JVM does it.
But I not time to dig into it

@Anonymitaet Anonymitaet added the doc-not-needed Your PR changes do not impact docs label Aug 4, 2021
@codelipenghui codelipenghui added this to the 2.9.0 milestone Aug 5, 2021
@codelipenghui codelipenghui merged commit 27855f8 into apache:master Aug 5, 2021
codelipenghui pushed a commit that referenced this pull request Aug 6, 2021
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Aug 6, 2021
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Oct 29, 2021
@Technoboy- Technoboy- deleted the replace-orElse-with-orElseGet branch August 10, 2022 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-2.8 Archived: 2.8 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants