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

Cleanup redundant allocations and code around Comparator use #13795

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

original-brownbear
Copy link
Member

Noticed some visible allocations in CompetitiveImpactAccumulator during benchmarking and fixed the needless allocation for the comparator in that class as well as a couple other similar spots where needless classes and/or objects could easily be replaced by more lightweight solutions.
Also inlined some single use comparator constants where a class turned into a non-capturing lambda for easier readability.

Noticed some visible allocations in CompetitiveImpactAccumulator
during benchmarking and fixed the needless allocation for the comparator
in that class as well as a couple other similar spots where needless
classes and/or objects could easily be replaced by more lightweight
solutions.
Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

Regardless of allocation, I like that this makes the code more readable in most cases by moving the comparison logic where it's used.

@jpountz jpountz merged commit 644feeb into apache:main Sep 17, 2024
4 checks passed
@jpountz jpountz added this to the 9.12.0 milestone Sep 17, 2024
jpountz pushed a commit that referenced this pull request Sep 17, 2024
Noticed some visible allocations in CompetitiveImpactAccumulator
during benchmarking and fixed the needless allocation for the comparator
in that class as well as a couple other similar spots where needless
classes and/or objects could easily be replaced by more lightweight
solutions.
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.

2 participants