Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

System.Linq.Enumerable has special treatment of IList<T>, which should also apply to IReadOnlyList<T> #57822

Closed
AlexVallat opened this issue Aug 20, 2021 · 4 comments
Labels
area-System.Linq untriaged New issue has not been triaged by the area owner

Comments

@AlexVallat
Copy link

In several places in System.Linq.Enumerable, it checks for a more performant path by testing if the source is IList<T> (for example

if (source is IList<TSource> list)
)

This should ideally also test for IReadOnlyList<T>, as IList<T> does not inherit from IReadOnlyList<T>, and an implementer of IReadOnlyList<T> is equally valid for the fast paths here.

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Linq untriaged New issue has not been triaged by the area owner labels Aug 20, 2021
@ghost
Copy link

ghost commented Aug 20, 2021

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

Issue Details

In several places in System.Linq.Enumerable, it checks for a more performant path by testing if the source is IList<T> (for example

if (source is IList<TSource> list)
)

This should ideally also test for IReadOnlyList<T>, as IList<T> does not inherit from IReadOnlyList<T>, and an implementer of IReadOnlyList<T> is equally valid for the fast paths here.

Author: AlexVallat
Assignees: -
Labels:

area-System.Linq, untriaged

Milestone: -

@huoyaoyuan
Copy link
Member

#42254

@Wraith2
Copy link
Contributor

Wraith2 commented Aug 20, 2021

from @EgorBo in #42254 (comment)

I found quite a few complains or rejected attempts to optimize LINQ for IReadOnlyCollection:

#28651 - LINQ results implicit support for IReadOnlyCollection
#27517 - Performance: Make constructor List(IEnumerable collection) know about IReadOnlyCollection
#26679 - Linq ToDictionary() should presize for IReadOnlyCollection
#14366 - System.Linq performance improvement suggestions (mentions IROC<>)
#24793 - Respect IReadOnlyList in the BCL
#23910 - Add optimized path for IReadOnlyCollection/IReadOnlyList in System.Linq
#18714 - Consider checking for IReadOnlyCollection in Enumerable.ToArray
#27516 - Performance of LINQ .Any() - type check to leverage .Count property? (mentions IROC)
#27517 - Performance: Make constructor List(IEnumerable collection) know about IReadOnlyCollection
dotnet/corefx#28472 - Check for IReadOnlyCollection
#43001 - LINQ IEnumerable extension methods should add special case IReadOnlyCollection

@eiriktsarpalis
Copy link
Member

Duplicate of #24793. Closing in favor of #42254.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Linq untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

4 participants