Skip to content

Apply optimizations and fixes to Enumerable.Take(Range) - #48634

Merged
eiriktsarpalis merged 14 commits into
dotnet:mainfrom
eiriktsarpalis:enumerable-take-optimizations
Mar 1, 2021
Merged

Apply optimizations and fixes to Enumerable.Take(Range)#48634
eiriktsarpalis merged 14 commits into
dotnet:mainfrom
eiriktsarpalis:enumerable-take-optimizations

Conversation

@eiriktsarpalis

@eiriktsarpaliseiriktsarpalis commented Feb 23, 2021

Copy link
Copy Markdown
Member

Makes the following changes to the new Enumerable.Take(Range) implementation:

  • Separate SizeOpt and SpeedOpt implementations for non-fromEnd ranges.
  • Optimize fromEnd ranges using Enumerable.TryGetNonEnumeratedCount to obtain the source count.
  • Change enumerator disposal semantics to more closely follow the TakeLast and SkipLast implementations.
  • Finally, expresses the TakeLast and SkipLast in terms of the Take(Range) implementation.

Fix#48631.

@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @eiriktsarpalis
See info in area-owners.md if you want to be subscribed.

Issue Details

Makes the following changes to the new Enumerable.Take(Range) implementation:

  • Separate SizeOpt and SpeedOpt implementations for non-fromEnd ranges.
  • Optimize fromEnd ranges using Enumerable.TryGetNonEnumeratedCount to obtain the source count.
  • Change enumerator disposal semantics to more closely follow the TakeLast and SkipLast implementations.
  • Finally, expresses the TakeLast and SkipLast in terms of the Take(Range) implementation.
Author:eiriktsarpalis
Assignees:eiriktsarpalis
Labels:

area-System.Linq

Milestone:6.0.0

@eiriktsarpalis

Copy link
Copy Markdown
MemberAuthor

cc @Dixin

@eiriktsarpalis

Copy link
Copy Markdown
MemberAuthor

I've been investigating the seemingly unrelated test failure affecting mono debug builds. The issue is caused by xunit's generic test parameter resolution logic: the removal of the private SkipLastIterator method seems to result in incorrect generic parameters being resolved for certain generic theory parameters.

I have pushed a commit to demonstrate this and to further show that the failures are not due to a regression introduced by this PR, but rather a bug of xunit running over mono debug builds.

@eiriktsarpalis
eiriktsarpalisforce-pushed the enumerable-take-optimizations branch from 7c73ea3 to 0c06999CompareFebruary 26, 2021 15:26
@eiriktsarpalis

Copy link
Copy Markdown
MemberAuthor

Hi @stephentoub@layomia, this is ready for review, would you be able to take a look?

Comment threadsrc/libraries/System.Linq/src/System/Linq/Take.SpeedOpt.cs
Comment threadsrc/libraries/System.Linq/src/System/Linq/Take.cs Outdated
Comment threadsrc/libraries/System.Linq/src/System/Linq/Take.cs Outdated
Co-authored-by: Stephen Toub <stoub@microsoft.com>
Base automatically changed from master to mainMarch 1, 2021 09:08
@eiriktsarpalis
eiriktsarpalis merged commit f7308f0 into dotnet:mainMar 1, 2021
@eiriktsarpalis
eiriktsarpalis deleted the enumerable-take-optimizations branch March 1, 2021 13:56
@ghostghost locked as resolved and limited conversation to collaborators Mar 31, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Optimize the implementation of LINQ partitioning APIs

2 participants

@eiriktsarpalis@stephentoub