Skip to content

Commit

Permalink
Fix Interleave to prevent eager MoveNext calls
Browse files Browse the repository at this point in the history 
This is a squashed merge of PR #696 that fixes #694.
  • Loading branch information
@Orace @atifaziz
Orace authored and atifaziz committed Dec 12, 2019
1 parent 9fc8486 commit 52f93f4
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 96 deletions.
12 changes: 12 additions & 0 deletions MoreLinq.Test/InterleaveTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,18 @@ public void TestInterleaveIsLazy()
new BreakingSequence<int>().Interleave(new BreakingSequence<int>());
}

/// <summary>
/// Verify that interleaving do not call enumerators MoveNext method eagerly
/// </summary>
[Test]
public void TestInterleaveDoNoCallMoveNextEagerly()
{
var sequenceA = Enumerable.Range(1, 1);
var sequenceB = MoreEnumerable.From<int>(() => throw new TestException());

sequenceA.Interleave(sequenceB).Take(1).Consume();
}

/// <summary>
/// Verify that interleaving disposes those enumerators that it managed
/// to open successfully
Expand Down
116 changes: 20 additions & 96 deletions MoreLinq/Interleave.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ namespace MoreLinq
{
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;

public static partial class MoreEnumerable
Expand All @@ -45,33 +44,6 @@ public static partial class MoreEnumerable
/// <returns>A sequence of interleaved elements from all of the source sequences</returns>

public static IEnumerable<T> Interleave<T>(this IEnumerable<T> sequence, params IEnumerable<T>[] otherSequences)
{
return Interleave(sequence, ImbalancedInterleaveStrategy.Skip, otherSequences);
}

/// <summary>
/// Interleaves the elements of two or more sequences into a single sequence, applying the specified strategy when sequences are of unequal length
/// </summary>
/// <remarks>
/// Interleave combines sequences by visiting each in turn, and returning the first element of each, followed
/// by the second, then the third, and so on. So, for example:<br/>
/// <code><![CDATA[
/// {1,1,1}.Interleave( {2,2,2}, {3,3,3} ) => { 1,2,3,1,2,3,1,2,3 }
/// ]]></code>
/// This operator behaves in a deferred and streaming manner.<br/>
/// When sequences are of unequal length, this method will use the imbalance strategy specified to
/// decide how to continue interleaving the remaining sequences. See <see cref="ImbalancedInterleaveStrategy"/>
/// for more information.<br/>
/// The sequences are interleaved in the order that they appear in the <paramref name="otherSequences"/>
/// collection, with <paramref name="sequence"/> as the first sequence.
/// </remarks>
/// <typeparam name="T">The type of the elements of the source sequences</typeparam>
/// <param name="sequence">The first sequence in the interleave group</param>
/// <param name="imbalanceStrategy">Defines the behavior of the operator when sequences are of unequal length</param>
/// <param name="otherSequences">The other sequences in the interleave group</param>
/// <returns>A sequence of interleaved elements from all of the source sequences</returns>

static IEnumerable<T> Interleave<T>(this IEnumerable<T> sequence, ImbalancedInterleaveStrategy imbalanceStrategy, params IEnumerable<T>[] otherSequences)
{
if (sequence == null) throw new ArgumentNullException(nameof(sequence));
if (otherSequences == null) throw new ArgumentNullException(nameof(otherSequences));
Expand All @@ -82,88 +54,40 @@ static IEnumerable<T> Interleave<T>(this IEnumerable<T> sequence, ImbalancedInte
{
var sequences = new[] { sequence }.Concat(otherSequences);

// produce an iterator collection for all IEnumerable<T> instancess passed to us
var iterators = sequences.Select(e => e.GetEnumerator()).Acquire();
List<IEnumerator<T>> iteratorList = null;
// produce an enumerators collection for all IEnumerable<T> instances passed to us
var enumerators = sequences.Select(e => e.GetEnumerator()).Acquire();

try
{
iteratorList = new List<IEnumerator<T>>(iterators);
iterators = null;
var shouldContinue = true;
var consumedIterators = 0;
var iterCount = iteratorList.Count;

while (shouldContinue)
var hasNext = true;
while (hasNext)
{
// advance every iterator and verify a value exists to be yielded
for (var index = 0; index < iterCount; index++)
hasNext = false;
for (var i = 0; i < enumerators.Length; i++)
{
if (!iteratorList[index].MoveNext())
{
// check if all iterators have been consumed and we can terminate
// or if the imbalance strategy informs us that we MUST terminate
if (++consumedIterators == iterCount || imbalanceStrategy == ImbalancedInterleaveStrategy.Stop)
{
shouldContinue = false;
break;
}

iteratorList[index].Dispose(); // dispose the iterator sice we no longer need it

// otherwise, apply the imbalance strategy
switch (imbalanceStrategy)
{
case ImbalancedInterleaveStrategy.Pad:
var newIter = iteratorList[index] = Generate(default(T), x => default).GetEnumerator();
newIter.MoveNext();
break;

case ImbalancedInterleaveStrategy.Skip:
iteratorList.RemoveAt(index); // no longer visit this particular iterator
--iterCount; // reduce the expected number of iterators to visit
--index; // decrement iterator index to compensate for index shifting
--consumedIterators; // decrement consumer iterator count to stay in balance
break;
}
var enumerator = enumerators[i];
if (enumerator == null)
continue;

if (enumerator.MoveNext())
{
hasNext = true;
yield return enumerator.Current;
}
else
{
enumerators[i] = null;
enumerator.Dispose();
}
}

if (shouldContinue) // only if all iterators could be advanced
{
// yield the values of each iterator's current position
foreach (var iterator in iteratorList)
yield return iterator.Current;
}
}
}
finally
{
Debug.Assert(iteratorList != null || iterators != null);
foreach (var iter in (iteratorList ?? (IList<IEnumerator<T>>) iterators))
iter.Dispose();
foreach (var enumerator in enumerators)
enumerator?.Dispose();
}
}
}

/// <summary>
/// Defines the strategies available when Interleave is passed sequences of unequal length
/// </summary>
enum ImbalancedInterleaveStrategy
{
/// <summary>
/// Extends a sequence by padding its tail with default(T)
/// </summary>
Pad,
/// <summary>
/// Removes the sequence from the interleave set, and continues interleaving remaining sequences.
/// </summary>
Skip,
/// <summary>
/// Stops the interleave operation.
/// </summary>
Stop,
}
}
}

0 comments on commit 52f93f4

Please sign in to comment.