Revamp empty special-casing in LINQ #96602
Conversation
Enumerable.Empty used to return Array.Empty. Towards the beginning of .NET Core, LINQ was imbued with an internal "partition" concept for flowing more information around between operators, and as part of that, Empty was changed to return a singleton instance of a specialized partition implementation. The upside of this was that methods typed to return IPartition could return the same singleton as Empty. There are multiple downsides, however. For one, the whole IPartition concept is only built into a "speed-optimized" build of LINQ; builds that care more about size (e.g. browser) end up not having it, and thus Empty there ends up being Array.Empty, such that a different type ends up being returned based on the build, which is not ideal. Further, any paths that check for empty now effectively have two things to check for: the empty partition or an empty array, making those checks more expensive, if they're even done at all, or in some cases missing out on possible optimization. This is more pronounced today, now that `[]` with collection expressions will produce Array.Empty, and it'd be really nice if there wasn't a difference between Enumerable.Empty and `[]` assigned to `IEnumerable<T>`. This change puts Enumerable.Empty back to always being Array.Empty. The internal IPartition-based APIs that drove us to need the EmptyPartition are changed to just use null as an indication of empty. Places we were already checking for `is EmptyPartition` are changed to check for an empty array (if they weren't already), and other APIs that weren't checking at all now have a check if it makes sense to do so (I audited all of the APIs, and didn't include checks in ones where it could meaningfully affect semantics, e.g. a fast path that might cause us not to get an enumerator from a secondary enumerable input).
|
Tagging subscribers to this area: @dotnet/area-system-linq Issue DetailsEnumerable.Empty used to return Array.Empty. Towards the beginning of .NET Core, LINQ was imbued with an internal "partition" concept for flowing more information around between operators, and as part of that, Empty was changed to return a singleton instance of a specialized partition implementation. The upside of this was that methods typed to return IPartition could return the same singleton as Empty. There are multiple downsides, however. For one, the whole IPartition concept is only built into a "speed-optimized" build of LINQ; builds that care more about size (e.g. browser) end up not having it, and thus Empty there ends up being Array.Empty, such that a different type ends up being returned based on the build, which is not ideal. Further, any paths that check for empty now effectively have two things to check for: the empty partition or an empty array, making those checks more expensive, if they're even done at all, or in some cases missing out on possible optimization. This is more pronounced today, now that This change puts Enumerable.Empty back to always being Array.Empty. The internal IPartition-based APIs that drove us to need the EmptyPartition are changed to just use null as an indication of empty. Places we were already checking for
|
The tests were validating the underlying type of the operator returned for Concat, which is not material.
The check needs to be moved to before the count <= 0 check... otherwise calling Skip on an empty array with a count of 0 would still allocate an iterator.
Yup. Fixed. |
Enumerable.Empty used to return Array.Empty. Towards the beginning of .NET Core, LINQ was imbued with an internal "partition" concept for flowing more information around between operators, and as part of that, Empty was changed to return a singleton instance of a specialized partition implementation. The upside of this was that methods typed to return IPartition could return the same singleton as Empty. There are multiple downsides, however. For one, the whole IPartition concept is only built into a "speed-optimized" build of LINQ; builds that care more about size (e.g. browser) end up not having it, and thus Empty there ends up being Array.Empty, such that a different type ends up being returned based on the build, which is not ideal. Further, any paths that check for empty now effectively have two things to check for: the empty partition or an empty array, making those checks more expensive, if they're even done at all, or in some cases missing out on possible optimization. This is more pronounced today, now that
[]with collection expressions will produce Array.Empty, and it'd be really nice if there wasn't a difference between Enumerable.Empty and[]assigned toIEnumerable<T>.This change puts Enumerable.Empty back to always being Array.Empty. The internal IPartition-based APIs that drove us to need the EmptyPartition are changed to just use null as an indication of empty. Places we were already checking for
is EmptyPartitionare changed to check for an empty array (if they weren't already), and other APIs that weren't checking at all now have a check if it makes sense to do so (I audited all of the APIs, and didn't include checks in ones where it could meaningfully affect semantics, e.g. a fast path that might cause us not to get an enumerator from a secondary enumerable input).