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

Optimize Enumerable.Range(...).Select(...) #37410

Merged
stephentoub merged 2 commits into from
May 6, 2019
Merged

Optimize Enumerable.Range(...).Select(...) #37410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14ab3ad
Optimize Enumerable.Range(...).Select(...)
stephentoub May 3, 2019
98824af
Add more Debug.Asserts
stephentoub May 6, 2019
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
2 changes: 1 addition & 1 deletion src/System.Linq/src/System/Linq/Range.SpeedOpt.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ private sealed partial class RangeIterator : IPartition<int>
{
public override IEnumerable<TResult> Select<TResult>(Func<int, TResult> selector)
{
return new SelectIPartitionIterator<int, TResult>(this, selector);
return new SelectRangeIterator<TResult>(_start, _end, selector);
}

public int[] ToArray()
Expand Down
5 changes: 5 additions & 0 deletions src/System.Linq/src/System/Linq/Repeat.SpeedOpt.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System.Collections.Generic;
using System.Diagnostics;

namespace System.Linq
{
Expand Down Expand Up @@ -39,6 +40,8 @@ public List<TResult> ToList()

public IPartition<TResult> Skip(int count)
{
Debug.Assert(count > 0);

if (count >= _count)
{
return EmptyPartition<TResult>.Instance;
Expand All @@ -49,6 +52,8 @@ public IPartition<TResult> Skip(int count)

public IPartition<TResult> Take(int count)
{
Debug.Assert(count > 0);

if (count >= _count)
{
return this;
Expand Down
157 changes: 149 additions & 8 deletions src/System.Linq/src/System/Linq/Select.SpeedOpt.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,13 @@ public IPartition<TResult> Skip(int count)
return new SelectListPartitionIterator<TSource, TResult>(_source, _selector, count, int.MaxValue);
}

public IPartition<TResult> Take(int count) =>
count >= _source.Length ? (IPartition<TResult>)this : new SelectListPartitionIterator<TSource, TResult>(_source, _selector, 0, count - 1);
public IPartition<TResult> Take(int count)
{
Debug.Assert(count > 0);
return count >= _source.Length ?
(IPartition<TResult>)this :
new SelectListPartitionIterator<TSource, TResult>(_source, _selector, 0, count - 1);
}

public TResult TryGetElementAt(int index, out bool found)
{
Expand Down Expand Up @@ -157,6 +162,132 @@ public TResult TryGetLast(out bool found)
}
}

private sealed partial class SelectRangeIterator<TResult> : Iterator<TResult>, IPartition<TResult>
{
private readonly int _start;
private readonly int _end;
private readonly Func<int, TResult> _selector;

public SelectRangeIterator(int start, int end, Func<int, TResult> selector)
{
Debug.Assert(start < end);
Comment thread
stephentoub marked this conversation as resolved.
Debug.Assert((end - start) <= int.MaxValue);
Copy link
Copy Markdown
Contributor

@timandy timandy May 15, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

This expression is always true,even overflow.

Does it should be?

Debug.Assert(unchecked((uint)(end - start) <= (uint)int.MaxValue));

Copy link
Copy Markdown
Member Author

@stephentoub stephentoub May 16, 2019

Choose a reason for hiding this comment

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

Yup, it's missing the casts. We'd welcome a PR if you'd like to fix it. Thanks.

Debug.Assert(selector != null);

_start = start;
_end = end;
_selector = selector;
}

public override Iterator<TResult> Clone() =>
new SelectRangeIterator<TResult>(_start, _end, _selector);

public override bool MoveNext()
{
if (_state < 1 || _state == (_end - _start + 1))
Comment thread
stephentoub marked this conversation as resolved.
{
Dispose();
return false;
}

int index = _state++ - 1;
Comment thread
stephentoub marked this conversation as resolved.
Debug.Assert(_start < _end - index);
_current = _selector(_start + index);
Comment thread
stephentoub marked this conversation as resolved.
return true;
}

public override IEnumerable<TResult2> Select<TResult2>(Func<TResult, TResult2> selector) =>
Comment thread
stephentoub marked this conversation as resolved.
new SelectRangeIterator<TResult2>(_start, _end, CombineSelectors(_selector, selector));

public TResult[] ToArray()
{
var results = new TResult[_end - _start];
int srcIndex = _start;
for (int i = 0; i < results.Length; i++)
{
results[i] = _selector(srcIndex++);
}

return results;
}

public List<TResult> ToList()
{
var results = new List<TResult>(_end - _start);
for (int i = _start; i != _end; i++)
Comment thread
stephentoub marked this conversation as resolved.
{
results.Add(_selector(i));
Comment thread
stephentoub marked this conversation as resolved.
}

return results;
}

public int GetCount(bool onlyIfCheap)
{
// In case someone uses Count() to force evaluation of the selector,
// run it provided `onlyIfCheap` is false.
if (!onlyIfCheap)
{
for (int i = _start; i != _end; i++)
{
_selector(i);
}
}

return _end - _start;
}

public IPartition<TResult> Skip(int count)
{
Debug.Assert(count > 0);

if (count >= (_end - _start))
{
return EmptyPartition<TResult>.Instance;
}

return new SelectRangeIterator<TResult>(_start + count, _end, _selector);
}

public IPartition<TResult> Take(int count)
Comment thread
stephentoub marked this conversation as resolved.
{
Debug.Assert(count > 0);

if (count >= (_end - _start))
{
return this;
}

return new SelectRangeIterator<TResult>(_start, _start + count, _selector);
}

public TResult TryGetElementAt(int index, out bool found)
{
if ((uint)index < (uint)(_end - _start))
{
found = true;
return _selector(_start + index);
}

found = false;
return default;
}

public TResult TryGetFirst(out bool found)
Comment thread
adamsitnik marked this conversation as resolved.
{
Debug.Assert(_end > _start);
found = true;
return _selector(_start);
}

public TResult TryGetLast(out bool found)
{
Debug.Assert(_end > _start);
found = true;
return _selector(_end - 1);
}
}

private sealed partial class SelectListIterator<TSource, TResult> : IPartition<TResult>
{
public TResult[] ToArray()
Expand Down Expand Up @@ -212,8 +343,11 @@ public IPartition<TResult> Skip(int count)
return new SelectListPartitionIterator<TSource, TResult>(_source, _selector, count, int.MaxValue);
}

public IPartition<TResult> Take(int count) =>
new SelectListPartitionIterator<TSource, TResult>(_source, _selector, 0, count - 1);
public IPartition<TResult> Take(int count)
{
Debug.Assert(count > 0);
return new SelectListPartitionIterator<TSource, TResult>(_source, _selector, 0, count - 1);
}

public TResult TryGetElementAt(int index, out bool found)
{
Expand Down Expand Up @@ -308,8 +442,11 @@ public IPartition<TResult> Skip(int count)
return new SelectListPartitionIterator<TSource, TResult>(_source, _selector, count, int.MaxValue);
}

public IPartition<TResult> Take(int count) =>
new SelectListPartitionIterator<TSource, TResult>(_source, _selector, 0, count - 1);
public IPartition<TResult> Take(int count)
{
Debug.Assert(count > 0);
return new SelectListPartitionIterator<TSource, TResult>(_source, _selector, 0, count - 1);
}

public TResult TryGetElementAt(int index, out bool found)
{
Expand Down Expand Up @@ -413,8 +550,11 @@ public IPartition<TResult> Skip(int count)
return new SelectIPartitionIterator<TSource, TResult>(_source.Skip(count), _selector);
}

public IPartition<TResult> Take(int count) =>
new SelectIPartitionIterator<TSource, TResult>(_source.Take(count), _selector);
public IPartition<TResult> Take(int count)
{
Debug.Assert(count > 0);
return new SelectIPartitionIterator<TSource, TResult>(_source.Take(count), _selector);
}

public TResult TryGetElementAt(int index, out bool found)
{
Expand Down Expand Up @@ -579,6 +719,7 @@ public IPartition<TResult> Skip(int count)

public IPartition<TResult> Take(int count)
{
Debug.Assert(count > 0);
int maxIndex = _minIndexInclusive + count - 1;
return (uint)maxIndex >= (uint)_maxIndexInclusive ? this : new SelectListPartitionIterator<TSource, TResult>(_source, _selector, _minIndexInclusive, maxIndex);
}
Expand Down