Skip to content
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

Jump to bottom

Optimize containsAll on RichIterable. #1344 #1353

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Optimize containsAll on RichIterable. #1344 #1353

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 10 additions & 2 deletions eclipse-collections-api/src/main/java/org/eclipse/collections/api/RichIterable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,15 +297,23 @@ else if (inside.size() > 32 && !(inside instanceof SetIterable))
*
* @since 1.0
*/
boolean containsAllIterable(Iterable<?> source);
default boolean containsAllIterable(Iterable<?> source)
{
RichIterable<?> outside = this.toSet();
return StreamSupport.stream(source.spliterator(), false).allMatch(outside::contains);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is a set, then calling toSet() is unnecessary, and this implementation is slower than necessary. I think we should leave this abstract on RichIterable.


/**
* Returns true if all elements in source are contained in this collection.
*
* @see Collection#containsAll(Collection)
* @since 1.0
*/
boolean containsAll(Collection<?> source);
default boolean containsAll(Collection<?> source)
{
return source instanceof RichIterable ? this.containsAllIterable(source)
: source.stream().allMatch(this::contains);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand the conditional here. What does it matter if source is a RichIterable? It's being passed to a method that takes Iterable.

In addition, containsAllIterable() may convert this to a set, but if the collection is a plain one, not a RichIterable, then we don't convert anything to a set. Why the difference? If it's for performance, this inconsistency doesn't make sense to me.


/**
* Returns true if all elements in the specified var arg array are contained in this collection.
Expand Down
8 changes: 8 additions & 0 deletions eclipse-collections-api/src/main/java/org/eclipse/collections/api/bag/MutableBag.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

package org.eclipse.collections.api.bag;

import java.util.Collection;

import org.eclipse.collections.api.bag.primitive.MutableBooleanBag;
import org.eclipse.collections.api.bag.primitive.MutableByteBag;
import org.eclipse.collections.api.bag.primitive.MutableCharBag;
Expand Down Expand Up @@ -230,6 +232,12 @@ default <V> MutableBag<V> countByEach(Function<? super T, ? extends Iterable<V>>
@Override
<V> MutableBag<V> flatCollect(Function<? super T, ? extends Iterable<V>> function);

@Override
default boolean containsAll(Collection<?> source)
{
return MutableBagIterable.super.containsAll(source);
}

/**
* @since 9.2
*/
Expand Down
8 changes: 8 additions & 0 deletions ...pse-collections-api/src/main/java/org/eclipse/collections/api/bag/MutableBagIterable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

package org.eclipse.collections.api.bag;

import java.util.Collection;

import org.eclipse.collections.api.RichIterable;
import org.eclipse.collections.api.block.function.Function;
import org.eclipse.collections.api.block.function.primitive.ObjectIntToObjectFunction;
Expand Down Expand Up @@ -43,6 +45,12 @@ public interface MutableBagIterable<T> extends Bag<T>, MutableCollection<T>
*/
int addOccurrences(T item, int occurrences);

@Override
default boolean containsAll(Collection<?> source)
{
return MutableCollection.super.containsAll(source);
}

boolean removeOccurrences(Object item, int occurrences);

boolean setOccurrences(T item, int occurrences);
Expand Down
8 changes: 8 additions & 0 deletions ...ollections-api/src/main/java/org/eclipse/collections/api/bag/sorted/MutableSortedBag.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

package org.eclipse.collections.api.bag.sorted;

import java.util.Collection;

import org.eclipse.collections.api.bag.MutableBag;
import org.eclipse.collections.api.bag.MutableBagIterable;
import org.eclipse.collections.api.block.function.Function;
Expand Down Expand Up @@ -49,6 +51,12 @@
public interface MutableSortedBag<T>
extends SortedBag<T>, MutableBagIterable<T>, Cloneable
{
@Override
default boolean containsAll(Collection<?> source)
{
return MutableBagIterable.super.containsAll(source);
}

@Override
MutableSortedBag<T> selectByOccurrences(IntPredicate predicate);

Expand Down
6 changes: 6 additions & 0 deletions ...ections-api/src/main/java/org/eclipse/collections/api/collection/FixedSizeCollection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,12 @@ public interface FixedSizeCollection<T>
@Override
boolean retainAllIterable(Iterable<?> iterable);

@Override
default boolean containsAll(Collection<?> source)
{
return MutableCollection.super.containsAll(source);
}

/**
* @throws UnsupportedOperationException the {@code clear} method is not supported by this collection.
*/
Expand Down
6 changes: 6 additions & 0 deletions ...llections-api/src/main/java/org/eclipse/collections/api/collection/MutableCollection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,12 @@ default <V> MutableMap<V, T> groupByUniqueKey(Function<? super T, ? extends V> f
*/
boolean retainAllIterable(Iterable<?> iterable);

@Override
default boolean containsAll(Collection<?> source)
{
return RichIterable.super.containsAll(source);
}

@Override
default <K, V> MutableMap<K, V> aggregateInPlaceBy(
Function<? super T, ? extends K> groupBy,
Expand Down
7 changes: 7 additions & 0 deletions eclipse-collections-api/src/main/java/org/eclipse/collections/api/list/FixedSizeList.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

package org.eclipse.collections.api.list;

import java.util.Collection;
import java.util.Comparator;

import org.eclipse.collections.api.block.procedure.Procedure;
Expand Down Expand Up @@ -41,6 +42,12 @@ public interface FixedSizeList<T>
@Override
FixedSizeList<T> tap(Procedure<? super T> procedure);

@Override
default boolean containsAll(Collection<?> source)
{
return MutableList.super.containsAll(source);
}

@Override
default FixedSizeList<T> sortThis(Comparator<? super T> comparator)
{
Expand Down
7 changes: 7 additions & 0 deletions eclipse-collections-api/src/main/java/org/eclipse/collections/api/list/MutableList.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

package org.eclipse.collections.api.list;

import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
Expand Down Expand Up @@ -394,6 +395,12 @@ default MutableList<T> shuffleThis(Random random)
return this;
}

@Override
default boolean containsAll(Collection<?> source)
{
return MutableCollection.super.containsAll(source);
}

/**
* Converts the MutableList to the default ImmutableList implementation.
*
Expand Down
8 changes: 8 additions & 0 deletions eclipse-collections-api/src/main/java/org/eclipse/collections/api/set/FixedSizeSet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

package org.eclipse.collections.api.set;

import java.util.Collection;

import org.eclipse.collections.api.block.procedure.Procedure;
import org.eclipse.collections.api.collection.FixedSizeCollection;

Expand All @@ -33,4 +35,10 @@ public interface FixedSizeSet<T>

@Override
FixedSizeSet<T> tap(Procedure<? super T> procedure);

@Override
default boolean containsAll(Collection<?> source)
{
return FixedSizeCollection.super.containsAll(source);
}
}
8 changes: 8 additions & 0 deletions eclipse-collections-api/src/main/java/org/eclipse/collections/api/set/MultiReaderSet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

package org.eclipse.collections.api.set;

import java.util.Collection;

import org.eclipse.collections.api.block.procedure.Procedure;

/**
Expand All @@ -23,4 +25,10 @@ public interface MultiReaderSet<T>
void withReadLockAndDelegate(Procedure<? super MutableSet<T>> procedure);

void withWriteLockAndDelegate(Procedure<? super MutableSet<T>> procedure);

@Override
default boolean containsAll(Collection<?> source)
{
return MutableSet.super.containsAll(source);
}
}
8 changes: 8 additions & 0 deletions eclipse-collections-api/src/main/java/org/eclipse/collections/api/set/MutableSet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

package org.eclipse.collections.api.set;

import java.util.Collection;

import org.eclipse.collections.api.block.function.Function;
import org.eclipse.collections.api.block.function.Function2;
import org.eclipse.collections.api.block.function.primitive.BooleanFunction;
Expand Down Expand Up @@ -196,6 +198,12 @@ default <P, V> MutableSet<V> flatCollectWith(Function2<? super T, ? super P, ? e
@Override
MutableSet<UnsortedSetIterable<T>> powerSet();

@Override
default boolean containsAll(Collection<?> source)
{
return MutableSetIterable.super.containsAll(source);
}

/**
* Converts the MutableSet to the default ImmutableSet implementation.
*
Expand Down
7 changes: 7 additions & 0 deletions ...pse-collections-api/src/main/java/org/eclipse/collections/api/set/MutableSetIterable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

package org.eclipse.collections.api.set;

import java.util.Collection;
import java.util.Set;

import org.eclipse.collections.api.block.function.Function;
Expand Down Expand Up @@ -98,4 +99,10 @@ default MutableSetIterable<T> withoutAll(Iterable<? extends T> elements)
this.removeAllIterable(elements);
return this;
}

@Override
default boolean containsAll(Collection<?> source)
{
return MutableCollection.super.containsAll(source);
}
}
7 changes: 7 additions & 0 deletions ...ollections-api/src/main/java/org/eclipse/collections/api/set/sorted/MutableSortedSet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

package org.eclipse.collections.api.set.sorted;

import java.util.Collection;
import java.util.SortedSet;

import org.eclipse.collections.api.block.function.Function;
Expand Down Expand Up @@ -237,6 +238,12 @@ default <P, V> MutableList<V> flatCollectWith(Function2<? super T, ? super P, ?
@Override
MutableSortedSet<T> tailSet(T fromElement);

@Override
default boolean containsAll(Collection<?> source)
{
return MutableSetIterable.super.containsAll(source);
}

/**
* @since 11.0
*/
Expand Down
10 changes: 10 additions & 0 deletions unit-tests/src/test/java/org/eclipse/collections/impl/AbstractRichIterableTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,12 @@ public void containsAllIterable()
{
RichIterable<Integer> collection = this.newWith(1, 2, 3, 4);
Assert.assertTrue(collection.containsAllIterable(Lists.mutable.with(1, 2)));
Assert.assertTrue(collection.containsAllIterable(Lists.mutable.with(1, 1, 2, 2, 2, 3)));
Assert.assertFalse(collection.containsAllIterable(Lists.mutable.with(1, 5)));
Assert.assertTrue(Interval.oneTo(100).containsAllIterable(collection));
Assert.assertFalse(Interval.fromTo(2, 100).containsAllIterable(collection));
Assert.assertTrue(this.newWith(Interval.oneTo(100).toArray()).containsAllIterable(Interval.oneTo(50)));
Assert.assertFalse(this.newWith(Interval.fromTo(5, 100).toArray()).containsAllIterable(Interval.fromTo(200, 250)));
}

@Test
Expand Down Expand Up @@ -309,6 +314,11 @@ public void containsAllCollection()
RichIterable<Integer> collection = this.newWith(1, 2, 3, 4);
Assert.assertTrue(collection.containsAll(Lists.mutable.with(1, 2)));
Assert.assertFalse(collection.containsAll(Lists.mutable.with(1, 5)));
Assert.assertFalse(collection.containsAll(Arrays.asList(1, 2, 5)));
Assert.assertTrue(collection.containsAll(Interval.oneTo(4)));
Assert.assertFalse(collection.containsAll(Interval.fromTo(5, 100)));
Assert.assertTrue(this.newWith(Interval.oneTo(100).toArray()).containsAll(Interval.oneTo(50)));
Assert.assertFalse(this.newWith(Interval.fromTo(5, 100).toArray()).containsAll(Interval.fromTo(50, 150)));
}

@Test
Expand Down