Skip to content

Commit

Permalink
Store each result in a "Present<V>" instance instead of an Optional<V>.
Browse files Browse the repository at this point in the history 
We can't store a plain V because we need to distinguish between Optional.absent() and null (at least in the Google-internal successfulAsMap -- and eventually in whenAllComplete(...).collectToList() (#1519), which is likely to omit failures instead of mapping them to null as successfulAsList does).

We may have problems using Optional when we adopt new nullability annotations, since Optional<V> might not be a valid instantiation when V is instantiated with a nullable type.

RELNOTES=n/a

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=298398456
  • Loading branch information
@cpovirk @nick-someone
cpovirk authored and nick-someone committed Mar 4, 2020
1 parent a8107fa commit e371bf1
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 22 deletions.
31 changes: 20 additions & 11 deletions android/guava/src/com/google/common/util/concurrent/CollectionFuture.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,17 @@
import static java.util.Collections.unmodifiableList;

import com.google.common.annotations.GwtCompatible;
import com.google.common.base.Optional;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import java.util.List;
import java.util.concurrent.Future;
import org.checkerframework.checker.nullness.compatqual.NullableDecl;

/** Aggregate future that collects (stores) results of each future. */
@GwtCompatible(emulated = true)
abstract class CollectionFuture<V, C> extends AggregateFuture<V, C> {
private List<Optional<V>> values;
private List<Present<V>> values;

CollectionFuture(
ImmutableCollection<? extends ListenableFuture<? extends V>> futures,
Expand All @@ -37,8 +37,8 @@ abstract class CollectionFuture<V, C> extends AggregateFuture<V, C> {

this.values =
futures.isEmpty()
? ImmutableList.<Optional<V>>of()
: Lists.<Optional<V>>newArrayListWithCapacity(futures.size());
? ImmutableList.<Present<V>>of()
: Lists.<Present<V>>newArrayListWithCapacity(futures.size());

// Populate the results list with null initially.
for (int i = 0; i < futures.size(); ++i) {
Expand All @@ -48,15 +48,15 @@ abstract class CollectionFuture<V, C> extends AggregateFuture<V, C> {

@Override
final void collectOneValue(int index, @NullableDecl V returnValue) {
List<Optional<V>> localValues = values;
List<Present<V>> localValues = values;
if (localValues != null) {
localValues.set(index, Optional.fromNullable(returnValue));
localValues.set(index, new Present<>(returnValue));
}
}

@Override
final void handleAllCompleted() {
List<Optional<V>> localValues = values;
List<Present<V>> localValues = values;
if (localValues != null) {
set(combine(localValues));
}
Expand All @@ -68,7 +68,7 @@ void releaseResources(ReleaseResourcesReason reason) {
this.values = null;
}

abstract C combine(List<Optional<V>> values);
abstract C combine(List<Present<V>> values);

/** Used for {@link Futures#allAsList} and {@link Futures#successfulAsList}. */
static final class ListFuture<V> extends CollectionFuture<V, List<V>> {
Expand All @@ -80,12 +80,21 @@ static final class ListFuture<V> extends CollectionFuture<V, List<V>> {
}

@Override
public List<V> combine(List<Optional<V>> values) {
public List<V> combine(List<Present<V>> values) {
List<V> result = newArrayListWithCapacity(values.size());
for (Optional<V> element : values) {
result.add(element != null ? element.orNull() : null);
for (Present<V> element : values) {
result.add(element != null ? element.value : null);
}
return unmodifiableList(result);
}
}

/** The result of a successful {@code Future}. */
private static final class Present<V> {
V value;

Present(V value) {
this.value = value;
}
}
}
31 changes: 20 additions & 11 deletions guava/src/com/google/common/util/concurrent/CollectionFuture.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,17 @@
import static java.util.Collections.unmodifiableList;

import com.google.common.annotations.GwtCompatible;
import com.google.common.base.Optional;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import java.util.List;
import java.util.concurrent.Future;
import org.checkerframework.checker.nullness.qual.Nullable;

/** Aggregate future that collects (stores) results of each future. */
@GwtCompatible(emulated = true)
abstract class CollectionFuture<V, C> extends AggregateFuture<V, C> {
private List<Optional<V>> values;
private List<Present<V>> values;

CollectionFuture(
ImmutableCollection<? extends ListenableFuture<? extends V>> futures,
Expand All @@ -37,8 +37,8 @@ abstract class CollectionFuture<V, C> extends AggregateFuture<V, C> {

this.values =
futures.isEmpty()
? ImmutableList.<Optional<V>>of()
: Lists.<Optional<V>>newArrayListWithCapacity(futures.size());
? ImmutableList.<Present<V>>of()
: Lists.<Present<V>>newArrayListWithCapacity(futures.size());

// Populate the results list with null initially.
for (int i = 0; i < futures.size(); ++i) {
Expand All @@ -48,15 +48,15 @@ abstract class CollectionFuture<V, C> extends AggregateFuture<V, C> {

@Override
final void collectOneValue(int index, @Nullable V returnValue) {
List<Optional<V>> localValues = values;
List<Present<V>> localValues = values;
if (localValues != null) {
localValues.set(index, Optional.fromNullable(returnValue));
localValues.set(index, new Present<>(returnValue));
}
}

@Override
final void handleAllCompleted() {
List<Optional<V>> localValues = values;
List<Present<V>> localValues = values;
if (localValues != null) {
set(combine(localValues));
}
Expand All @@ -68,7 +68,7 @@ void releaseResources(ReleaseResourcesReason reason) {
this.values = null;
}

abstract C combine(List<Optional<V>> values);
abstract C combine(List<Present<V>> values);

/** Used for {@link Futures#allAsList} and {@link Futures#successfulAsList}. */
static final class ListFuture<V> extends CollectionFuture<V, List<V>> {
Expand All @@ -80,12 +80,21 @@ static final class ListFuture<V> extends CollectionFuture<V, List<V>> {
}

@Override
public List<V> combine(List<Optional<V>> values) {
public List<V> combine(List<Present<V>> values) {
List<V> result = newArrayListWithCapacity(values.size());
for (Optional<V> element : values) {
result.add(element != null ? element.orNull() : null);
for (Present<V> element : values) {
result.add(element != null ? element.value : null);
}
return unmodifiableList(result);
}
}

/** The result of a successful {@code Future}. */
private static final class Present<V> {
V value;

Present(V value) {
this.value = value;
}
}
}

0 comments on commit e371bf1

Please sign in to comment.