Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Optimize Enumerable.Skip() for IList<> parameter #4551

Merged
VSadov merged 3 commits into from
Jan 4, 2016
Merged

Optimize Enumerable.Skip() for IList<> parameter #4551

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5a72312
Optimize Enumerable.Skip for IList.
Nov 17, 2015
c9953f4
Avoid repeated interface call as suggested by @stephentoub.
Nov 17, 2015
7d775ac
Revert "Avoid repeated interface call as suggested by @stephentoub."
Nov 19, 2015
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
17 changes: 16 additions & 1 deletion src/System.Linq/src/System/Linq/Enumerable.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -884,7 +884,22 @@ private static IEnumerable<TSource> TakeWhileIterator<TSource>(IEnumerable<TSour
public static IEnumerable<TSource> Skip<TSource>(this IEnumerable<TSource> source, int count)
{
if (source == null) throw Error.ArgumentNull("source");
return SkipIterator<TSource>(source, count);

IList<TSource> sourceList = source as IList<TSource>;
return sourceList != null ? SkipList(sourceList, count) : SkipIterator<TSource>(source, count);
}

private static IEnumerable<TSource> SkipList<TSource>(IList<TSource> source, int count)
{
if (count < 0)
{
count = 0;
}

while (count < source.Count)
Copy link
Copy Markdown
Member

@stephentoub stephentoub Nov 17, 2015

Choose a reason for hiding this comment

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

It'd be good to cache source.Count into a "local"; otherwise this will incur an interface call on each iteration. The extra field for that localon the display class is likely a good tradeoff.

Copy link
Copy Markdown
Member

@bartonjs bartonjs Jan 4, 2016

Choose a reason for hiding this comment

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

Since there was contention about saving the Count value to a local (yes: save on interface invocation, no: what if the list was mutated) I think we should have a test that fires one of these up, iterates a bit, removes some data from the original IList value, and continues iterating. That way the most-recently agreed-upon behavior is codified in a test that goes in along with the change.

{
yield return source[count++];
}
}

private static IEnumerable<TSource> SkipIterator<TSource>(IEnumerable<TSource> source, int count)
Expand Down