Perf Issue: toImmutable[List/Set/Bag] methods should be overridden on LazyIterable

[donraab]

The default implementations of toImmutable[List/Set/Bag] are suboptimal for LazyIterable. Calls to isEmpty or size should be avoided for LazyIterable as they will force iteration to happen. The following tests will fail currently and should be added with appropriate fixes, which should probably be just overriding with default methods in LazyIterable.

@Test
public void perfIssueInLazyIterableToImmutableList()
{
    MutableBag<Integer> visited = Bags.mutable.empty();
    LazyIterable<Integer> integers = Interval.oneTo(10);
    LazyIterable<Integer> select =
            integers.select(i -> i > 3)
                    .tap(visited::add)
                    .select(i -> i < 7)
                    .tap(visited::add);
    ImmutableList<Integer> immutableList = select.toImmutableList();

    Assertions.assertEquals(List.of(4, 5, 6), immutableList);

    // Expected: [4, 4, 5, 5, 6, 6, 7, 8, 9, 10]
    // Actual: [4, 4, 4, 4, 5, 5, 6, 6, 7, 8, 9, 10, 4, 4, 5, 5, 6, 6, 7, 8, 9, 10]
    Assertions.assertEquals(Bags.mutable.of(4, 4, 5, 5, 6, 6, 7, 8, 9, 10), visited);
}

@Test
public void perfIssueInLazyIterableToImmutableBag()
{
    MutableBag<Integer> visited = Bags.mutable.empty();
    LazyIterable<Integer> integers = Interval.oneTo(10).toBag().asLazy();
    LazyIterable<Integer> select =
            integers.select(i -> i > 3)
                    .tap(visited::add)
                    .select(i -> i < 7)
                    .tap(visited::add);
    Bag<Integer> immutableBag = select.toImmutableBag();

    Assertions.assertEquals(Bags.mutable.of(4, 5, 6), immutableBag);

    // Expected: [4, 4, 5, 5, 6, 6, 7, 8, 9, 10]
    // Actual: [4, 4, 4, 4, 4, 4, 5, 5, 5, 5, 6, 6, 6, 6, 7, 7, 8, 8, 9, 9, 10, 10]
    Assertions.assertEquals(Bags.mutable.of(4, 4, 5, 5, 6, 6, 7, 8, 9, 10), visited);
}

@Test
public void perfIssueInLazyIterableToImmutableSet()
{
    MutableBag<Integer> visited = Bags.mutable.empty();
    LazyIterable<Integer> integers = Interval.oneTo(10).toBag().asLazy();
    LazyIterable<Integer> select =
            integers.select(i -> i > 3)
                    .tap(visited::add)
                    .select(i -> i < 7)
                    .tap(visited::add);
    ImmutableSet<Integer> immutableBag = select.toImmutableSet();

    Assertions.assertEquals(Set.of(4, 5, 6), immutableBag);

    // Expected: [4, 4, 5, 5, 6, 6, 7, 8, 9, 10]
    // Actual: [4, 4, 4, 4, 4, 4, 5, 5, 5, 5, 6, 6, 6, 6, 7, 7, 8, 8, 9, 9, 10, 10]
    Assertions.assertEquals(Bags.mutable.of(4, 4, 5, 5, 6, 6, 7, 8, 9, 10), visited);
}

[fermendivilt]

Hello, I would like to work on this issue!
Since there is no one assigned, I will start working on it and report back when solved,,, or if I get stuck :)

[fermendivilt]

To avoid confusion with my hidden comment: Tomorrow I will create a pull request with a solution for the issue.
Good night to anyone reading this c:

[motlin]

I was going to say that the implementation should use toArray()

    /**
     * Converts the LazyIterable to the default ImmutableList implementation.
     */
    @Override
    default ImmutableList<T> toImmutableList()
    {
        T[] array = (T[]) this.toArray();
        return Lists.immutable.with(array);
    }

    /**
     * Converts the LazyIterable to the default ImmutableSet implementation.
     */
    @Override
    default ImmutableSet<T> toImmutableSet()
    {
        T[] array = (T[]) this.toArray();
        return Sets.immutable.with(array);
    }

    /**
     * Converts the LazyIterable to the default ImmutableBag implementation.
     */
    @Override
    default ImmutableBag<T> toImmutableBag()
    {
        T[] array = (T[]) this.toArray();
        return Bags.immutable.with(array);
    }

However I see that toArray() has the same problem and needs to be fixed too.

    @Override
    public Object[] toArray()
    {
        Object[] result = new Object[this.size()];
        this.forEachWithIndex((each, index) -> result[index] = each);
        return result;
    }