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.

Sign up for GitHub

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

Jump to bottom

Remove a bunch of array allocations #82041

Merged
merged 1 commit into from
Feb 14, 2023
Merged

Remove a bunch of array allocations #82041

merged 1 commit into from
Feb 14, 2023

Conversation

stephentoub
Copy link
Member

A bunch of array allocations being removed by:

  • Changing static readonly arrays to be spans (taking advantage of RuntimeHelpers.CreateSpan)
  • Changing to use stackalloc
  • Changing interop to use pointers rather than arrays
  • Using ArrayPool
  • Computing a string at dev time rather than at run time
  • Lazy initializing the array when it's only needed on a fallback path
  • Deleting an array that's never used
  • Using Unsafe.AsRef to workaround Encoding not liking empty arrays

@ghost ghost assigned stephentoub Feb 13, 2023
@ghost
Copy link

ghost commented Feb 13, 2023

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

Issue Details

A bunch of array allocations being removed by:

  • Changing static readonly arrays to be spans (taking advantage of RuntimeHelpers.CreateSpan)
  • Changing to use stackalloc
  • Changing interop to use pointers rather than arrays
  • Using ArrayPool
  • Computing a string at dev time rather than at run time
  • Lazy initializing the array when it's only needed on a fallback path
  • Deleting an array that's never used
  • Using Unsafe.AsRef to workaround Encoding not liking empty arrays
Author: stephentoub
Assignees: stephentoub
Labels:

area-Meta
Milestone: -

tannergooding
tannergooding approved these changes Feb 13, 2023
View reviewed changes
Copy link
Member

@tannergooding tannergooding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes generally LGTM. Just a couple comments/nits/suggestions

@build-analysis build-analysis bot mentioned this pull request Feb 13, 2023
Closed
@stephentoub stephentoub merged commit 5584b26 into dotnet:main Feb 14, 2023
@stephentoub stephentoub deleted the removemorearrays branch February 14, 2023 17:31
@cincuranet cincuranet mentioned this pull request Feb 16, 2023
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Mar 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@am11 am11 am11 left review comments

@tannergooding tannergooding tannergooding approved these changes

@lewing lewing Awaiting requested review from lewing lewing is a code owner

@pavelsavara pavelsavara Awaiting requested review from pavelsavara pavelsavara is a code owner

Assignees

@stephentoub stephentoub

Labels
area-Meta
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@stephentoub @am11 @tannergooding