Skip to content

[7.x] Optimize allocations for List.copyOf and Set.copyOf #79395

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 19, 2021
Merged

[7.x] Optimize allocations for List.copyOf and Set.copyOf #79395

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c03d1f0
Optimize allocations for List.copyOf and Set.copyOf
arteam Oct 11, 2021
36bef69
Use Collections directly
arteam Oct 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions libs/core/src/main/java/org/elasticsearch/core/List.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

package org.elasticsearch.core;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -75,8 +76,14 @@ public static <T> java.util.List<T> of(T... entries) {
* @param coll a {@code Collection} from which elements are drawn, must be non-null
* @return a {@code List} containing the elements of the given {@code Collection}
*/
@SuppressWarnings("unchecked")
public static <T> java.util.List<T> copyOf(Collection<? extends T> coll) {
return (java.util.List<T>) List.of(coll.toArray());
switch (coll.size()) {
case 0:
return Collections.emptyList();
case 1:
return Collections.singletonList(coll.iterator().next());
default:
return Collections.unmodifiableList(new ArrayList<>(coll));
}
}
}
14 changes: 10 additions & 4 deletions libs/core/src/main/java/org/elasticsearch/core/Set.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ public static <T> java.util.Set<T> of(T e1, T e2) {
public static <T> java.util.Set<T> of(T... entries) {
switch (entries.length) {
case 0:
return Set.of();
return of();
case 1:
return Set.of(entries[0]);
return of(entries[0]);
default:
return Collections.unmodifiableSet(new HashSet<>(Arrays.asList(entries)));
}
Expand All @@ -78,8 +78,14 @@ public static <T> java.util.Set<T> of(T... entries) {
* @throws NullPointerException if coll is null, or if it contains any nulls
* @since 10
*/
@SuppressWarnings("unchecked")
public static <T> java.util.Set<T> copyOf(Collection<? extends T> coll) {
return (java.util.Set<T>) Set.of(new HashSet<>(coll).toArray());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one looks especially bad, because we allocate a HashSet just to call toArray which exists for Collection.

switch (coll.size()) {
case 0:
return Collections.emptySet();
case 1:
return Collections.singleton(coll.iterator().next());
default:
return Collections.unmodifiableSet(new HashSet<>(coll));
}
}
}