You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/09/01 18:32:13 UTC
[GitHub] [arrow] laurentgo opened a new pull request #11063: ARROW-13847: [Java] Avoid unnecessary collection copies
laurentgo opened a new pull request #11063:
URL: https://github.com/apache/arrow/pull/11063
There are several occurences in Arrow Java codebase where temporary
collections are being created and immediately discarded like when going
over a sequence of elements, or when making a copy of a collection
provided as an argument to a constructor.
This change adds a couple of methods to Collections2 and AutoCloseables,
along with some changes to the codebase to remove those extra copies.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] laurentgo commented on a change in pull request #11063: ARROW-13847: [Java] Avoid unnecessary collection copies
Posted by GitBox <gi...@apache.org>.
laurentgo commented on a change in pull request #11063:
URL: https://github.com/apache/arrow/pull/11063#discussion_r700668677
##########
File path: java/memory/memory-core/src/main/java/org/apache/arrow/util/Collections2.java
##########
@@ -46,23 +45,55 @@ private Collections2() {}
* Converts the iterable into a new {@link List}.
*/
public static <T> List<T> toList(Iterable<T> iterable) {
- return StreamSupport.stream(iterable.spliterator(), false).collect(Collectors.toList());
+ if (iterable instanceof Collection<?>) {
+ // If iterable is a collection, take advantage of it for a more efficient copy
+ return new ArrayList<T>((Collection<T>) iterable);
+ }
+ return toList(iterable.iterator());
}
+ /**
+ * Converts the iterable into a new immutable {@link List}.
+ */
+ public static <T> List<T> toImmutableList(Iterable<T> iterable) {
+ return Collections.unmodifiableList(toList(iterable));
+ }
+
+
/** Copies the elements of <code>map</code> to a new unmodifiable map. */
public static <K, V> Map<K, V> immutableMapCopy(Map<K, V> map) {
- Map<K, V> newMap = new HashMap<>();
- newMap.putAll(map);
- return java.util.Collections.unmodifiableMap(newMap);
+ return Collections.unmodifiableMap(new HashMap<>(map));
}
/** Copies the elements of list to a new unmodifiable list. */
public static <V> List<V> immutableListCopy(List<V> list) {
- return Collections.unmodifiableList(list.stream().collect(Collectors.toList()));
+ return Collections.unmodifiableList(new ArrayList<>(list));
}
/** Copies the values to a new unmodifiable list. */
public static <V> List<V> asImmutableList(V...values) {
return Collections.unmodifiableList(Arrays.asList(values));
}
+
+ /**
+ * Creates a human readable string from the remaining elements in iterator.
+ *
+ * The output should be similar to {@code Arrays#toString(Object[])}
+ */
+ public static String toString(Iterator<?> iterator) {
+ if (!iterator.hasNext()) {
Review comment:
Added a commit to change it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jacques-n commented on a change in pull request #11063: ARROW-13847: [Java] Avoid unnecessary collection copies
Posted by GitBox <gi...@apache.org>.
jacques-n commented on a change in pull request #11063:
URL: https://github.com/apache/arrow/pull/11063#discussion_r700651265
##########
File path: java/memory/memory-core/src/main/java/org/apache/arrow/util/Collections2.java
##########
@@ -46,23 +45,55 @@ private Collections2() {}
* Converts the iterable into a new {@link List}.
*/
public static <T> List<T> toList(Iterable<T> iterable) {
- return StreamSupport.stream(iterable.spliterator(), false).collect(Collectors.toList());
+ if (iterable instanceof Collection<?>) {
+ // If iterable is a collection, take advantage of it for a more efficient copy
+ return new ArrayList<T>((Collection<T>) iterable);
+ }
+ return toList(iterable.iterator());
}
+ /**
+ * Converts the iterable into a new immutable {@link List}.
+ */
+ public static <T> List<T> toImmutableList(Iterable<T> iterable) {
+ return Collections.unmodifiableList(toList(iterable));
Review comment:
Sad. Any chance we can source something small that already does that (in a separate jira)? Otherwise I expect we'll probably need another patch here in the future since we're still probably doing excessive copies.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jacques-n commented on a change in pull request #11063: ARROW-13847: [Java] Avoid unnecessary collection copies
Posted by GitBox <gi...@apache.org>.
jacques-n commented on a change in pull request #11063:
URL: https://github.com/apache/arrow/pull/11063#discussion_r700650320
##########
File path: java/memory/memory-core/src/main/java/org/apache/arrow/util/Collections2.java
##########
@@ -46,23 +45,55 @@ private Collections2() {}
* Converts the iterable into a new {@link List}.
*/
public static <T> List<T> toList(Iterable<T> iterable) {
- return StreamSupport.stream(iterable.spliterator(), false).collect(Collectors.toList());
+ if (iterable instanceof Collection<?>) {
+ // If iterable is a collection, take advantage of it for a more efficient copy
+ return new ArrayList<T>((Collection<T>) iterable);
+ }
+ return toList(iterable.iterator());
}
+ /**
+ * Converts the iterable into a new immutable {@link List}.
+ */
+ public static <T> List<T> toImmutableList(Iterable<T> iterable) {
+ return Collections.unmodifiableList(toList(iterable));
+ }
+
+
/** Copies the elements of <code>map</code> to a new unmodifiable map. */
public static <K, V> Map<K, V> immutableMapCopy(Map<K, V> map) {
- Map<K, V> newMap = new HashMap<>();
- newMap.putAll(map);
- return java.util.Collections.unmodifiableMap(newMap);
+ return Collections.unmodifiableMap(new HashMap<>(map));
}
/** Copies the elements of list to a new unmodifiable list. */
public static <V> List<V> immutableListCopy(List<V> list) {
- return Collections.unmodifiableList(list.stream().collect(Collectors.toList()));
+ return Collections.unmodifiableList(new ArrayList<>(list));
}
/** Copies the values to a new unmodifiable list. */
public static <V> List<V> asImmutableList(V...values) {
return Collections.unmodifiableList(Arrays.asList(values));
}
+
+ /**
+ * Creates a human readable string from the remaining elements in iterator.
+ *
+ * The output should be similar to {@code Arrays#toString(Object[])}
+ */
+ public static String toString(Iterator<?> iterator) {
+ if (!iterator.hasNext()) {
Review comment:
Up to you whether worth changing. I figured that might be slightly clearer...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] laurentgo commented on a change in pull request #11063: ARROW-13847: [Java] Avoid unnecessary collection copies
Posted by GitBox <gi...@apache.org>.
laurentgo commented on a change in pull request #11063:
URL: https://github.com/apache/arrow/pull/11063#discussion_r700668610
##########
File path: java/memory/memory-core/src/main/java/org/apache/arrow/util/Collections2.java
##########
@@ -46,23 +45,55 @@ private Collections2() {}
* Converts the iterable into a new {@link List}.
*/
public static <T> List<T> toList(Iterable<T> iterable) {
- return StreamSupport.stream(iterable.spliterator(), false).collect(Collectors.toList());
+ if (iterable instanceof Collection<?>) {
+ // If iterable is a collection, take advantage of it for a more efficient copy
+ return new ArrayList<T>((Collection<T>) iterable);
+ }
+ return toList(iterable.iterator());
}
+ /**
+ * Converts the iterable into a new immutable {@link List}.
+ */
+ public static <T> List<T> toImmutableList(Iterable<T> iterable) {
+ return Collections.unmodifiableList(toList(iterable));
Review comment:
Not to knowledge
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on pull request #11063: ARROW-13847: [Java] Avoid unnecessary collection copies
Posted by GitBox <gi...@apache.org>.
emkornfield commented on pull request #11063:
URL: https://github.com/apache/arrow/pull/11063#issuecomment-917709739
+1 will merge on green.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jacques-n commented on a change in pull request #11063: ARROW-13847: [Java] Avoid unnecessary collection copies
Posted by GitBox <gi...@apache.org>.
jacques-n commented on a change in pull request #11063:
URL: https://github.com/apache/arrow/pull/11063#discussion_r700629594
##########
File path: java/memory/memory-core/src/main/java/org/apache/arrow/util/Collections2.java
##########
@@ -46,23 +45,55 @@ private Collections2() {}
* Converts the iterable into a new {@link List}.
*/
public static <T> List<T> toList(Iterable<T> iterable) {
- return StreamSupport.stream(iterable.spliterator(), false).collect(Collectors.toList());
+ if (iterable instanceof Collection<?>) {
+ // If iterable is a collection, take advantage of it for a more efficient copy
+ return new ArrayList<T>((Collection<T>) iterable);
+ }
+ return toList(iterable.iterator());
}
+ /**
+ * Converts the iterable into a new immutable {@link List}.
+ */
+ public static <T> List<T> toImmutableList(Iterable<T> iterable) {
+ return Collections.unmodifiableList(toList(iterable));
+ }
+
+
/** Copies the elements of <code>map</code> to a new unmodifiable map. */
public static <K, V> Map<K, V> immutableMapCopy(Map<K, V> map) {
- Map<K, V> newMap = new HashMap<>();
- newMap.putAll(map);
- return java.util.Collections.unmodifiableMap(newMap);
+ return Collections.unmodifiableMap(new HashMap<>(map));
}
/** Copies the elements of list to a new unmodifiable list. */
public static <V> List<V> immutableListCopy(List<V> list) {
- return Collections.unmodifiableList(list.stream().collect(Collectors.toList()));
+ return Collections.unmodifiableList(new ArrayList<>(list));
}
/** Copies the values to a new unmodifiable list. */
public static <V> List<V> asImmutableList(V...values) {
return Collections.unmodifiableList(Arrays.asList(values));
}
+
+ /**
+ * Creates a human readable string from the remaining elements in iterator.
+ *
+ * The output should be similar to {@code Arrays#toString(Object[])}
+ */
+ public static String toString(Iterator<?> iterator) {
+ if (!iterator.hasNext()) {
Review comment:
Isn't there a better stream-y way to do this with join?
##########
File path: java/vector/src/main/java/org/apache/arrow/vector/types/pojo/Schema.java
##########
@@ -97,7 +97,7 @@ public static Schema convertSchema(org.apache.arrow.flatbuf.Schema schema) {
String value = kv.value();
metadata.put(key == null ? "" : key, value == null ? "" : value);
}
- return new Schema(Collections2.immutableListCopy(fields), Collections2.immutableMapCopy(metadata));
+ return new Schema(true, Collections.unmodifiableList(fields), Collections.unmodifiableMap(metadata));
Review comment:
You're skipping the defensive copy here? Seems like a change in behavior.
##########
File path: java/vector/src/main/java/org/apache/arrow/vector/types/pojo/FieldType.java
##########
@@ -68,11 +69,9 @@ public FieldType(boolean nullable, ArrowType type, DictionaryEncoding dictionary
extensionMetadata.put(ExtensionType.EXTENSION_METADATA_KEY_NAME, ((ExtensionType) type).extensionName());
extensionMetadata.put(ExtensionType.EXTENSION_METADATA_KEY_METADATA, ((ExtensionType) type).serialize());
if (metadata != null) {
- for (Map.Entry<String, String> entry : metadata.entrySet()) {
- extensionMetadata.put(entry.getKey(), entry.getValue());
- }
+ extensionMetadata.putAll(metadata);
}
- this.metadata = Collections2.immutableMapCopy(extensionMetadata);
+ this.metadata = Collections.unmodifiableMap(extensionMetadata);
Review comment:
You're removing the copy here. Is this safe?
##########
File path: java/memory/memory-core/src/main/java/org/apache/arrow/util/Collections2.java
##########
@@ -46,23 +45,55 @@ private Collections2() {}
* Converts the iterable into a new {@link List}.
*/
public static <T> List<T> toList(Iterable<T> iterable) {
- return StreamSupport.stream(iterable.spliterator(), false).collect(Collectors.toList());
+ if (iterable instanceof Collection<?>) {
+ // If iterable is a collection, take advantage of it for a more efficient copy
+ return new ArrayList<T>((Collection<T>) iterable);
+ }
+ return toList(iterable.iterator());
}
+ /**
+ * Converts the iterable into a new immutable {@link List}.
+ */
+ public static <T> List<T> toImmutableList(Iterable<T> iterable) {
+ return Collections.unmodifiableList(toList(iterable));
Review comment:
Can we specialize this so it avoids a copy if the existing set is unmodifiable with a safe backing. Same with the unmodifiable map below. Seems like you could then keep the defensive copies you've removed below in Schema and FieldType
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #11063: ARROW-13847: [Java] Avoid unnecessary collection copies
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11063:
URL: https://github.com/apache/arrow/pull/11063#issuecomment-910551870
https://issues.apache.org/jira/browse/ARROW-13847
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield closed pull request #11063: ARROW-13847: [Java] Avoid unnecessary collection copies
Posted by GitBox <gi...@apache.org>.
emkornfield closed pull request #11063:
URL: https://github.com/apache/arrow/pull/11063
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] laurentgo commented on a change in pull request #11063: ARROW-13847: [Java] Avoid unnecessary collection copies
Posted by GitBox <gi...@apache.org>.
laurentgo commented on a change in pull request #11063:
URL: https://github.com/apache/arrow/pull/11063#discussion_r700647478
##########
File path: java/vector/src/main/java/org/apache/arrow/vector/types/pojo/FieldType.java
##########
@@ -68,11 +69,9 @@ public FieldType(boolean nullable, ArrowType type, DictionaryEncoding dictionary
extensionMetadata.put(ExtensionType.EXTENSION_METADATA_KEY_NAME, ((ExtensionType) type).extensionName());
extensionMetadata.put(ExtensionType.EXTENSION_METADATA_KEY_METADATA, ((ExtensionType) type).serialize());
if (metadata != null) {
- for (Map.Entry<String, String> entry : metadata.entrySet()) {
- extensionMetadata.put(entry.getKey(), entry.getValue());
- }
+ extensionMetadata.putAll(metadata);
}
- this.metadata = Collections2.immutableMapCopy(extensionMetadata);
+ this.metadata = Collections.unmodifiableMap(extensionMetadata);
Review comment:
because `extensionMetadata` is being by the constructor itself and not exposed else, I believe this to be safe, yes.
##########
File path: java/memory/memory-core/src/main/java/org/apache/arrow/util/Collections2.java
##########
@@ -46,23 +45,55 @@ private Collections2() {}
* Converts the iterable into a new {@link List}.
*/
public static <T> List<T> toList(Iterable<T> iterable) {
- return StreamSupport.stream(iterable.spliterator(), false).collect(Collectors.toList());
+ if (iterable instanceof Collection<?>) {
+ // If iterable is a collection, take advantage of it for a more efficient copy
+ return new ArrayList<T>((Collection<T>) iterable);
+ }
+ return toList(iterable.iterator());
}
+ /**
+ * Converts the iterable into a new immutable {@link List}.
+ */
+ public static <T> List<T> toImmutableList(Iterable<T> iterable) {
+ return Collections.unmodifiableList(toList(iterable));
+ }
+
+
/** Copies the elements of <code>map</code> to a new unmodifiable map. */
public static <K, V> Map<K, V> immutableMapCopy(Map<K, V> map) {
- Map<K, V> newMap = new HashMap<>();
- newMap.putAll(map);
- return java.util.Collections.unmodifiableMap(newMap);
+ return Collections.unmodifiableMap(new HashMap<>(map));
}
/** Copies the elements of list to a new unmodifiable list. */
public static <V> List<V> immutableListCopy(List<V> list) {
- return Collections.unmodifiableList(list.stream().collect(Collectors.toList()));
+ return Collections.unmodifiableList(new ArrayList<>(list));
}
/** Copies the values to a new unmodifiable list. */
public static <V> List<V> asImmutableList(V...values) {
return Collections.unmodifiableList(Arrays.asList(values));
}
+
+ /**
+ * Creates a human readable string from the remaining elements in iterator.
+ *
+ * The output should be similar to {@code Arrays#toString(Object[])}
+ */
+ public static String toString(Iterator<?> iterator) {
+ if (!iterator.hasNext()) {
Review comment:
I guess something like
```
return StreamSupport.stream(Spliterators.spliteratorUnknownSize(iterator, Spliterator.ORDERED), false)
.map(String::valueOf)
.collect(Collectors.joining(", ", "[", "]"));
```
should work
##########
File path: java/memory/memory-core/src/main/java/org/apache/arrow/util/Collections2.java
##########
@@ -46,23 +45,55 @@ private Collections2() {}
* Converts the iterable into a new {@link List}.
*/
public static <T> List<T> toList(Iterable<T> iterable) {
- return StreamSupport.stream(iterable.spliterator(), false).collect(Collectors.toList());
+ if (iterable instanceof Collection<?>) {
+ // If iterable is a collection, take advantage of it for a more efficient copy
+ return new ArrayList<T>((Collection<T>) iterable);
+ }
+ return toList(iterable.iterator());
}
+ /**
+ * Converts the iterable into a new immutable {@link List}.
+ */
+ public static <T> List<T> toImmutableList(Iterable<T> iterable) {
+ return Collections.unmodifiableList(toList(iterable));
Review comment:
Unlike Guava `Immutable*` collections, there's no officlal way to inspect for a collection to check if the collection is unmodifiable. Probably the best way would be to introduce Arrow's own set of immutable collections, but that's a way bigger issue. In this patch I tried to only focus on the obvious copy duplicates
##########
File path: java/vector/src/main/java/org/apache/arrow/vector/types/pojo/Schema.java
##########
@@ -97,7 +97,7 @@ public static Schema convertSchema(org.apache.arrow.flatbuf.Schema schema) {
String value = kv.value();
metadata.put(key == null ? "" : key, value == null ? "" : value);
}
- return new Schema(Collections2.immutableListCopy(fields), Collections2.immutableMapCopy(metadata));
+ return new Schema(true, Collections.unmodifiableList(fields), Collections.unmodifiableMap(metadata));
Review comment:
Both defensive copies are done by the constructor itself when the original argument is processed to create a `fields` and a `metadata` collections
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] laurentgo commented on a change in pull request #11063: ARROW-13847: [Java] Avoid unnecessary collection copies
Posted by GitBox <gi...@apache.org>.
laurentgo commented on a change in pull request #11063:
URL: https://github.com/apache/arrow/pull/11063#discussion_r701339753
##########
File path: java/memory/memory-core/src/main/java/org/apache/arrow/util/AutoCloseables.java
##########
@@ -113,9 +114,23 @@ public static void close(Iterable<? extends AutoCloseable> ac) throws Exception
*/
@SafeVarargs
public static void close(Iterable<? extends AutoCloseable>...closeables) throws Exception {
- close(Arrays.asList(closeables).stream()
- .flatMap((Iterable<? extends AutoCloseable> t) -> Collections2.toList(t).stream())
- .collect(Collectors.toList()));
+ close(flatten(closeables));
+ }
+
+ @SafeVarargs
+ private static Iterable<AutoCloseable> flatten(Iterable<? extends AutoCloseable>... closeables) {
+ return new Iterable<AutoCloseable>() {
+ // Cast from Iterable<? extends AutoCloseable> to Iterable<AutoCloseable> is safe in this context
+ // since there's no modification of the original collection
+ @SuppressWarnings("unchecked")
+ @Override
+ public Iterator<AutoCloseable> iterator() {
+ return Arrays.stream(closeables)
+ .flatMap((Iterable<? extends AutoCloseable> i)
+ -> StreamSupport.stream(((Iterable<AutoCloseable>) i).spliterator(), false))
Review comment:
Hopefully addressed by latest commit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on a change in pull request #11063: ARROW-13847: [Java] Avoid unnecessary collection copies
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #11063:
URL: https://github.com/apache/arrow/pull/11063#discussion_r700718127
##########
File path: java/memory/memory-core/src/main/java/org/apache/arrow/util/AutoCloseables.java
##########
@@ -113,9 +114,23 @@ public static void close(Iterable<? extends AutoCloseable> ac) throws Exception
*/
@SafeVarargs
public static void close(Iterable<? extends AutoCloseable>...closeables) throws Exception {
- close(Arrays.asList(closeables).stream()
- .flatMap((Iterable<? extends AutoCloseable> t) -> Collections2.toList(t).stream())
- .collect(Collectors.toList()));
+ close(flatten(closeables));
+ }
+
+ @SafeVarargs
+ private static Iterable<AutoCloseable> flatten(Iterable<? extends AutoCloseable>... closeables) {
+ return new Iterable<AutoCloseable>() {
+ // Cast from Iterable<? extends AutoCloseable> to Iterable<AutoCloseable> is safe in this context
+ // since there's no modification of the original collection
+ @SuppressWarnings("unchecked")
+ @Override
+ public Iterator<AutoCloseable> iterator() {
+ return Arrays.stream(closeables)
+ .flatMap((Iterable<? extends AutoCloseable> i)
+ -> StreamSupport.stream(((Iterable<AutoCloseable>) i).spliterator(), false))
Review comment:
could you provide a parameter comment for the false literal?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on a change in pull request #11063: ARROW-13847: [Java] Avoid unnecessary collection copies
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #11063:
URL: https://github.com/apache/arrow/pull/11063#discussion_r706911128
##########
File path: java/memory/memory-core/src/main/java/org/apache/arrow/util/AutoCloseables.java
##########
@@ -113,9 +114,23 @@ public static void close(Iterable<? extends AutoCloseable> ac) throws Exception
*/
@SafeVarargs
public static void close(Iterable<? extends AutoCloseable>...closeables) throws Exception {
- close(Arrays.asList(closeables).stream()
- .flatMap((Iterable<? extends AutoCloseable> t) -> Collections2.toList(t).stream())
- .collect(Collectors.toList()));
+ close(flatten(closeables));
+ }
+
+ @SafeVarargs
+ private static Iterable<AutoCloseable> flatten(Iterable<? extends AutoCloseable>... closeables) {
+ return new Iterable<AutoCloseable>() {
+ // Cast from Iterable<? extends AutoCloseable> to Iterable<AutoCloseable> is safe in this context
+ // since there's no modification of the original collection
+ @SuppressWarnings("unchecked")
+ @Override
+ public Iterator<AutoCloseable> iterator() {
+ return Arrays.stream(closeables)
+ .flatMap((Iterable<? extends AutoCloseable> i)
+ -> StreamSupport.stream(((Iterable<AutoCloseable>) i).spliterator(), false))
Review comment:
```suggestion
-> StreamSupport.stream(((Iterable<AutoCloseable>) i).spliterator(), /*parallel=*/false))
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org