Skip to content

Performance Optimization: Replace String.join with Collectors.joining in NodeSelectionInvocationHandler.getNodeDescription #3261

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

Closed
ori0o0p opened this issue Apr 18, 2025 · 1 comment · Fixed by #3262
Labels
type: improvement An improvement to the existing implementation
Milestone

Comments

@ori0o0p
Copy link
Contributor

ori0o0p commented Apr 18, 2025

Feature Request

Is your feature request related to a problem? Please describe

The current implementation of getNodeDescription() method creates an unnecessary intermediate collection by using String.join() with Collectors.toList(). This can lead to decreased performance and increased memory usage, especially in high-throughput scenarios where this method is frequently called.

Describe the solution you'd like

Replace the current implementation:
https://github.com/redis/lettuce/blob/main/src/main/java/io/lettuce/core/cluster/NodeSelectionInvocationHandler.java

private String getNodeDescription(List<RedisClusterNode> notFinished) {
    return String.join(", ", notFinished.stream().map(this::getDescriptor).collect(Collectors.toList()));
}

With a more efficient approach using Collectors.joining():

private String getNodeDescription(List<RedisClusterNode> notFinished) {
    return notFinished.stream().map(this::getDescriptor).collect(Collectors.joining(", "));
}

This optimization eliminates the temporary list creation while maintaining identical functionality, resulting in better performance and reduced memory overhead.

Describe alternatives you've considered

  1. Keep the current implementation, as the performance impact might be minimal in some use cases
  2. Use a StringBuilder directly, but this would be more verbose and less idiomatic with the Stream API
  3. Create a custom collector, but this would be overly complex for this simple case

Teachability, Documentation, Adoption, Migration Strategy

This change is a straightforward optimization that maintains the same behavior and API. No documentation updates are needed as this is an internal implementation detail.

For developers working on the codebase, this change serves as a good example of using Stream collectors efficiently. Similar patterns could be applied to other string-joining operations throughout the codebase.

@tishun tishun added this to the 6.6.0.RELEASE milestone Apr 23, 2025
@tishun tishun added the type: improvement An improvement to the existing implementation label Apr 23, 2025
@tishun
Copy link
Collaborator

tishun commented Apr 23, 2025

Hey @ori0o0p that makes sense.

FWIW - in case you have other small optimizations like this one - you could simply make a PR without an issue.
We would like to have issues like this one in case we want to discuss a larger contribution.

The PR itself is enough for a smaller change.

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: improvement An improvement to the existing implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants