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

Reduce memory usage by Array.Empty #3085

Merged

Conversation

lindexi
Copy link
Member

@lindexi lindexi commented Jun 3, 2020

The Empty property are only used in TypeReferences

@lindexi lindexi requested a review from a team as a code owner June 3, 2020 11:33
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Jun 3, 2020
@ghost ghost requested review from fabiant3, ryalanms and SamBent June 3, 2020 11:33
@miloush
Copy link
Contributor

miloush commented Jun 3, 2020

Note that this is a public API now returning a fixed array instead of a list, though an array seems more appropriate here.

There is also System.Windows.Markup.Primitives.ElementPropertyBase.EmptyTypes.

@Rand-Random
Copy link

The title totally got my expectations high, but the actual code change destroyed my expectations pretty fast.

@lindexi lindexi changed the title Reduce memory usage Reduce memory usage by Array.Empty Jun 3, 2020
@vatsan-madhavan
Copy link
Member

vatsan-madhavan commented Jun 6, 2020

I was just looking up List yesterday for something else and noticed that new List() seems to have an optimization similar to Array.Empty() in that it returns an empty pre-constructed array by default.

            _items = s_emptyArray;

In situations where an empty List is converted again into an IReadOnlyList etc. that incurs another object allocation, the slight improvement of Array.Empty() is clearer.

        public ReadOnlyCollection<T> AsReadOnly()
            => new ReadOnlyCollection<T>(this);

But straight-up swapping of an empty List with Array.Empty() (esp. for cases where the desired interface type is implemented by either of those, like IEnumerable) - this I'm not sure I understand the benefit of.

@lindexi
Copy link
Member Author

lindexi commented Jun 7, 2020

@ vatsan-madhavan Yeah, but we create a static field will cause unnecessary memory allocation, although there is only very little memory.

  • a static field
  • a List object
    • object meta
    • _items field
    • _size field
    • _version field

@vatsan-madhavan
Copy link
Member

Makes sense.

lindexi added a commit to lindexi/lindexi_gd that referenced this pull request Nov 5, 2020
@ryalanms ryalanms merged commit 8c66feb into dotnet:master Jan 7, 2021
@lindexi lindexi deleted the t/lindexi/reduce-memory-usage branch January 7, 2021 01:40
@ghost ghost locked as resolved and limited conversation to collaborators Apr 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants