-
Notifications
You must be signed in to change notification settings - Fork 5k
Optimise First, Last, FirstOrDefault & LastOrDefault for OrderedEnumerable #2401
Conversation
6577304
to
9cb5db1
Compare
cc: @VSadov |
Fixes #2238 (And I think this is the last bit to separate out of the original PR) https://github.com/hackcraft/Enumerable-Tester/raw/master/Skip%20performance.ods shows a comparison of the old and new approaches. Improvements are made in the vast majority of cases, and with Skip in particular can be significant. Edge-cases of disposal and exception ordering are dealt with; duplicating the behaviour found with the current implementation. (This ignores the matter of ordered skips as per dotnet#2401).
Fixes #2238 (And I think this is the last bit to separate out of the original PR) https://github.com/hackcraft/Enumerable-Tester/raw/master/Skip%20performance.ods shows a comparison of the old and new approaches. Improvements are made in the vast majority of cases, and with Skip in particular can be significant. Edge-cases of disposal and exception ordering are dealt with; duplicating the behaviour found with the current implementation. (This ignores the matter of ordered skips as per dotnet#2401).
@@ -2958,7 +3136,67 @@ IEnumerator IEnumerable.GetEnumerator() | |||
} | |||
} | |||
|
|||
internal abstract class OrderedEnumerable<TElement> : IOrderedEnumerable<TElement> | |||
internal interface IOrderedPartitonable<TElement> : IEnumerable<TElement> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in interface name IOrderedPartit_i_onable
Fixes #2238 (And I think this is the last bit to separate out of the original PR) https://github.com/hackcraft/Enumerable-Tester/raw/master/Skip%20performance.ods and https://github.com/hackcraft/Enumerable-Tester/raw/master/new%20version%20skip%20improvements.ods show comparisons of the old and new approaches. Improvements are made in the vast majority of cases. (This ignores the matter of ordered skips as per dotnet#2401).
Fixes #2238 (And I think this is the last bit to separate out of the original PR) https://github.com/hackcraft/Enumerable-Tester/raw/master/Skip%20performance.ods and https://github.com/hackcraft/Enumerable-Tester/raw/master/new%20version%20skip%20improvements.ods show comparisons of the old and new approaches. Improvements are made in the vast majority of cases. (This ignores the matter of ordered skips as per dotnet#2401).
Fixes #2238 (And I think this is the last bit to separate out of the original PR) https://github.com/hackcraft/Enumerable-Tester/raw/master/Skip%20performance.ods and https://github.com/hackcraft/Enumerable-Tester/raw/master/new%20version%20skip%20improvements.ods show comparisons of the old and new approaches. Improvements are made in the vast majority of cases. (This ignores the matter of ordered skips as per dotnet#2401).
e7a89d7
to
bc8e638
Compare
Fixes #2238 (And I think this is the last bit to separate out of the original PR) https://github.com/hackcraft/Enumerable-Tester/raw/master/Skip%20performance.ods and https://github.com/hackcraft/Enumerable-Tester/raw/master/new%20version%20skip%20improvements.ods show comparisons of the old and new approaches. Improvements are made in the vast majority of cases. (This ignores the matter of ordered skips as per dotnet#2401). Also similar Take improvements. In particular, the common Linq paging Idiom of a Skip followed by a Take is catered for specifically.
bc8e638
to
6f1696b
Compare
@@ -122,11 +122,6 @@ public IEnumerator<TSource> GetEnumerator() | |||
|
|||
public abstract IEnumerable<TSource> Where(Func<TSource, bool> predicate); | |||
|
|||
public virtual TSource[] ToArray() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing this from here was mainly to be able to have both Iterator and IPartition share the same interface-path to ToArray. Some Iterators have a ToArray that ends up going through Buffer twice, so just removing them and letting Buffer do what it would do anyway is a side-effect benefit.
After some further changes, checks, and squashing, the state of this PR is: The following operations have changed time complexity on the results of an
There is also some work to make the worse-case less likely to be hit. When (
Once the results are prepared, enumerating (directly or indirectly) is also reduced from Θ(j + k) to Θ(k). These ordered
Those marked † also had a O(n) component that was completely removed, so the lower-order costs are also improved. All of these use a Θ(n) fast-path when possible, which is sometimes O(1) in space and sometimes O(n). Obtaining an array of all the above results is also optimsed. Obtaining an array of those iterators that can't special-case the operation themselves is also optimised as a side-effect of that. |
Which is a complexity not directly relevant and may not be worth it. I had been doing median-of-three, but I don't think that should be done here, so I took it out again. Whether it helps or hinders should probably be considered separately. |
@@ -605,6 +594,9 @@ public override TResult[] ToArray() | |||
public static IEnumerable<TSource> Take<TSource>(this IEnumerable<TSource> source, int count) | |||
{ | |||
if (source == null) throw Error.ArgumentNull("source"); | |||
if (count <= 0) return EmptyPartition<TSource>.Instance; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or perhaps Empty() would be better. Since GetEnumerator()
isn't hit in the original form, there's no meaningful observable difference this way, but there could be with Empty().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rect: It does affect garbage collection. That effect is a generally good one (faster collection and perhaps in an earlier generation), but it is an effect that can be observed with finalisers or weak references, so it's not a completely unobservable change.
@JonHanna, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR. |
4ce5df1
to
2dac0d1
Compare
2dac0d1
to
8b8aafa
Compare
Test Innerloop Windows_NT Debug Build and Test |
Optimise First, Last, FirstOrDefault & LastOrDefault for OrderedEnumerable
Optimisation of Skip() for IList sources from dotnet#4551 fits with optimisations of Skip() and Take() for other sources from dotnet#2401. Combine the approaches, extending how the result of Skip() on a list handles subsequent operations.
Optimisation of Skip() for IList sources from dotnet#4551 fits with optimisations of Skip() and Take() for other sources from dotnet#2401. Combine the approaches, extending how the result of Skip() on a list handles subsequent operations.
Optimisation of Skip() for IList sources from dotnet#4551 fits with optimisations of Skip() and Take() for other sources from dotnet#2401. Combine the approaches, extending how the result of Skip() on a list handles subsequent operations.
Optimisation of Skip() for IList sources from dotnet#4551 fits with optimisations of Skip() and Take() for other sources from dotnet#2401. Combine the approaches, extending how the result of Skip() on a list handles subsequent operations.
Anything that can serve as one can serve as the other, and also provide a faster path for Count(). Merge the two interfaces and add a Count property. Have IList optimised result of Skip() partitionable. Optimisation of Skip() for IList sources from dotnet#4551 fits with optimisations of Skip() and Take() for other sources from dotnet#2401. Combine the approaches, extending how the result of Skip() on a list handles subsequent operations.
Anything that can serve as one can serve as the other, and also provide a faster path for Count(). Merge the two interfaces and add a Count property. Have IList optimised result of Skip() partitionable. Optimisation of Skip() for IList sources from dotnet#4551 fits with optimisations of Skip() and Take() for other sources from dotnet#2401. Combine the approaches, extending how the result of Skip() on a list handles subsequent operations.
Anything that can serve as one can serve as the other, and also provide a faster path for Count(). Merge the two interfaces and add a Count property. Have IList optimised result of Skip() partitionable. Optimisation of Skip() for IList sources from dotnet#4551 fits with optimisations of Skip() and Take() for other sources from dotnet#2401. Combine the approaches, extending how the result of Skip() on a list handles subsequent operations.
Anything that can serve as one can serve as the other, and also provide a faster path for Count(). Merge the two interfaces and add a Count property. Have IList optimised result of Skip() partitionable. Optimisation of Skip() for IList sources from dotnet#4551 fits with optimisations of Skip() and Take() for other sources from dotnet#2401. Combine the approaches, extending how the result of Skip() on a list handles subsequent operations.
Anything that can serve as one can serve as the other, and also provide a faster path for Count(). Merge the two interfaces and add a Count property. Have IList optimised result of Skip() partitionable. Optimisation of Skip() for IList sources from dotnet#4551 fits with optimisations of Skip() and Take() for other sources from dotnet#2401. Combine the approaches, extending how the result of Skip() on a list handles subsequent operations.
Anything that can serve as one can serve as the other, and also provide a faster path for Count(). Merge the two interfaces and add a Count property. Have IList optimised result of Skip() partitionable. Optimisation of Skip() for IList sources from dotnet#4551 fits with optimisations of Skip() and Take() for other sources from dotnet#2401. Combine the approaches, extending how the result of Skip() on a list handles subsequent operations.
Anything that can serve as one can serve as the other, and also provide a faster path for Count(). Merge the two interfaces and add a Count property. Have IList optimised result of Skip() partitionable. Optimisation of Skip() for IList sources from dotnet#4551 fits with optimisations of Skip() and Take() for other sources from dotnet#2401. Combine the approaches, extending how the result of Skip() on a list handles subsequent operations.
Anything that can serve as one can serve as the other, and also provide a faster path for Count(). Merge the two interfaces and add a Count property. Have IList optimised result of Skip() partitionable. Optimisation of Skip() for IList sources from dotnet#4551 fits with optimisations of Skip() and Take() for other sources from dotnet#2401. Combine the approaches, extending how the result of Skip() on a list handles subsequent operations.
Anything that can serve as one can serve as the other, and also provide a faster path for Count(). Merge the two interfaces and add a Count property. Have IList optimised result of Skip() partitionable. Optimisation of Skip() for IList sources from dotnet#4551 fits with optimisations of Skip() and Take() for other sources from dotnet#2401. Combine the approaches, extending how the result of Skip() on a list handles subsequent operations.
Anything that can serve as one can serve as the other, and also provide a faster path for Count(). Merge the two interfaces and add a Count property. Have IList optimised result of Skip() partitionable. Optimisation of Skip() for IList sources from dotnet#4551 fits with optimisations of Skip() and Take() for other sources from dotnet#2401. Combine the approaches, extending how the result of Skip() on a list handles subsequent operations.
Anything that can serve as one can serve as the other, and also provide a faster path for Count(). Merge the two interfaces and add a Count property. Have IList optimised result of Skip() partitionable. Optimisation of Skip() for IList sources from dotnet#4551 fits with optimisations of Skip() and Take() for other sources from dotnet#2401. Combine the approaches, extending how the result of Skip() on a list handles subsequent operations.
pulled |
Optimise First, Last, FirstOrDefault & LastOrDefault for OrderedEnumerable Commit migrated from dotnet/corefx@b2126e1
Anything that can serve as one can serve as the other, and also provide a faster path for Count(). Merge the two interfaces and add a Count property. Have IList optimised result of Skip() partitionable. Optimisation of Skip() for IList sources from dotnet/corefx#4551 fits with optimisations of Skip() and Take() for other sources from dotnet/corefx#2401. Combine the approaches, extending how the result of Skip() on a list handles subsequent operations. Commit migrated from dotnet/corefx@a087c2d
Fixes #2400
If you only want the first or last element of an ordered enumerable, you do
not need to O(n log n) (worse case, O(n²) but rare) operation of sorting
the entire set in O(n) space, but can instead to an O(n) retrieval of just
that element in O(1) space.