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

Use Enumerable.Empty and Array.Empty #4654

Merged
merged 1 commit into from Mar 17, 2018

Conversation

drewnoakes
Copy link
Member

Enumerable.Empty and Array.Empty return singleton instances, so reduce overall allocations.

Also use new [] { ... } over new List<T> { ... } in a few places, for lesser allocations. The latter allocates an over-sized array (capacity 16) as well as the overhead of List<T>.

In a few places I substitute in IReadOnlyList<T>. I feel like GE could use this interface more broadly, though understand the interface itself is newer than much of the codebase. I will use it more in future.

What did I do to test the code and ensure quality:

  • Manual review of straightforward changes. The compiler does the checking here.

These return singleton instances, so reduce overall allocations.

Also use `new []` over `new List` in a few places, for lesser allocations.
@vbjay
Copy link
Contributor

vbjay commented Mar 17, 2018 via email

@codecov
Copy link

codecov bot commented Mar 17, 2018

Codecov Report

Merging #4654 into master will not change coverage.
The diff coverage is 31.25%.

@@           Coverage Diff           @@
##           master    #4654   +/-   ##
=======================================
  Coverage   26.68%   26.68%           
=======================================
  Files         501      501           
  Lines       40752    40752           
  Branches     5961     5963    +2     
=======================================
  Hits        10873    10873           
  Misses      29379    29379           
  Partials      500      500
Impacted Files Coverage Δ
...ommandsDialogs/RepoHosting/ViewPullRequestsForm.cs 9.44% <ø> (ø) ⬆️
GitCommands/Git/GitModule.cs 8.54% <0%> (ø) ⬆️
GitCommands/Config/ConfigFile.cs 80.71% <0%> (ø) ⬆️
GitUI/UserControls/RevisionGrid.cs 8.69% <0%> (ø) ⬆️
ResourceManager/GitPluginBase.cs 0% <0%> (ø) ⬆️
ResourceManager/Translator.cs 28.2% <0%> (ø) ⬆️
...ntrols/ToolStripClasses/CommitIconProviderTests.cs 100% <100%> (ø) ⬆️
...ols/AuthorEmailBasedRevisionHighlightingFixture.cs 100% <100%> (ø) ⬆️
GitCommands/ExternalLinks/ExternalLinksLoader.cs 81.48% <50%> (ø) ⬆️
...UI/CommandsDialogs/BrowseDialog/FormBrowseMenus.cs 77.9% <50%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9949704...d0bb0b6. Read the comment docs.

@drewnoakes
Copy link
Member Author

Depends on if you are going to remove or add items. Also there is toarray on ienumerable.

You cannot add items to an empty IEnumerable<T> or a T[]. That is why these framework methods return singletons -- they cannot be mutated.

Calling Enumerable.ToArray will allocate you a new array. The point of this optimisation is to avoid redundant allocations.

@vbjay
Copy link
Contributor

vbjay commented Mar 17, 2018 via email

@RussKie RussKie merged commit 8bff40b into gitextensions:master Mar 17, 2018
@RussKie RussKie added this to the 3.00 milestone Mar 17, 2018
@drewnoakes drewnoakes deleted the empty-collections branch March 18, 2018 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants