Avoiding some duplicated work in Async #3023
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Async.each(list, cfFactory) with factory was delegating back to Async.each(futures), allocating some extra objects (CompletableFuture + List). The current implementation simply delegates to the new Async.CombinedBuilder.
bbakerman
reviewed
Nov 21, 2022
| }); | ||
| return overallResult; | ||
| public static <U> CompletableFuture<List<U>> each(List<CompletableFuture<U>> futures) { // TODO: used only in tests now | ||
| CombinedBuilder<U> overall = ofExpectedSize(futures.size()); |
Member
There was a problem hiding this comment.
You could also created a method that produced a build of the right size in one call
CombinedBuilder<U> overall = fromList(futures);
There is no need to add them all one by one outside that method.
You can do the same code as here inside the fromList method say
Contributor
Author
There was a problem hiding this comment.
I tried that but I stopped for 2 reasons:
- that will defeat the idea of the
CombinedBuilder(i.e. avoid List -> array conversion/allocations) - this method is used only for tests now... and since it is
@Internal, we should be fine no?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@bbakerman @andimarek hello :)
I'm trying to reduce some duplications in Async class. What do you think?