mirror of
https://github.com/jellyfin/jellyfin.git
synced 2026-07-16 05:43:54 +03:00
ToList Usage #2692
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @shortspider on GitHub (Mar 3, 2021).
Describe the bug
In looking at the code I noticed that
ToListis used quite liberally. Take the following as an example (but there are lots more):The problem with the above is that every call to
ToListcreates a copy of theIEnumerable. So in the above we create a copy of_profiles.Values(this may or may not be needed depending on thread safety) and we then create a another copy after we order and filter the first copy. This second copy at least isn't needed at all since the method states that it returns anIEnumerable. This copying is death by 1000 cuts since we need to spend CPU allocating the memory for the copy, doing the copy, and then collecting the original and copy when the GC kicks in.There are a number of usages like this in the code. I'd like to take a crack at fixing them but wanted to know how best to go about it. One big PR? A PR per class or namespace? I understand this isn't exactly a bug report but it's not a feature request either so I thought I'd post here. Please feel free to close and direct me somewhere else if that's more appropriate.
@crobibero commented on GitHub (Mar 3, 2021):
I would recommend a class per class/namespace, large PRs tend to stick around as they’re less likely to be reviewed
@dkanada commented on GitHub (Mar 3, 2021):
I'd say LoC is a better indicator of review turnaround time. Anything under 200 LoC is usually pretty fast here so if you can fit everything within that size it could be fine :p
@cvium commented on GitHub (Mar 3, 2021):
Your example is not terribly great. The first
.ToList()is probably superfluous given the lock on the_profilesobject. But the last.ToList()is necessary. LINQ is evaluated lazily, so you have to force the enumeration inside the lock.@shortspider commented on GitHub (Mar 3, 2021):
@cvium I mean, you still are pointing out that one
ToListis unneeded so I think it serves as a decent example. The lock does complicate it though.@crobibero @dkanada thanks for the feedback, I'll try and go by class/namespace while keeping the LoC down.
@stale[bot] commented on GitHub (Jul 1, 2021):
This issue has gone 120 days without comment. To avoid abandoned issues, it will be closed in 21 days if there are no new comments.
If you're the original submitter of this issue, please comment confirming if this issue still affects you in the latest release or nightlies, or close the issue if it has been fixed. If you're another user also affected by this bug, please comment confirming so. Either action will remove the stale label.
This bot exists to prevent issues from becoming stale and forgotten. Jellyfin is always moving forward, and bugs are often fixed as side effects of other changes. We therefore ask that bug report authors remain vigilant about their issues to ensure they are closed if fixed, or re-confirmed - perhaps with fresh logs or reproduction examples - regularly. If you have any questions you can reach us on Matrix or Social Media.
@Bond-009 commented on GitHub (Jul 1, 2021):
@cvium Do we want to keep this open? All the biggest allocations should already be fixed, no?
@cvium commented on GitHub (Jul 1, 2021):
The ToList usage is still a bit excessive in some places, so dunno
@jellyfin-bot commented on GitHub (Oct 24, 2023):
This issue has gone 120 days without an update and will be closed within 21 days if there is no new activity. To prevent this issue from being closed, please confirm the issue has not already been fixed by providing updated examples or logs.
If you have any questions you can use one of several ways to contact us.
@jellyfin-bot commented on GitHub (Nov 14, 2023):
This issue was closed due to inactivity.
@carmosilva-vinicius commented on GitHub (Nov 23, 2024):
I'm interested in tackling this issue as a good-first-issue. Here are my thoughts on how to approach it:
Would this be an appropriate good-first-issue for a newcomer to the project? Are there any specific areas of the codebase that would be good places to start?
@felix920506 commented on GitHub (Nov 24, 2024):
Please head to the development channel in our chat rooms to discuss the matter further. https://jellyfin.org/contact
@sarah-strawberries commented on GitHub (Nov 21, 2025):
My classmates @happystar9 and @jesus-sunuc and I are interested in helping out with this; any input on what we should get started with?
@sjorge commented on GitHub (Nov 21, 2025):
Felix's message above is still valid, hop over to the development chat on matrix or discord and ask there. It can take bit for someone to reply with timezones and all being a thing.