Skip to content
New issue

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

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

Already on GitHub? # to your account

Remove bunch of allocations caused by static arrays by favoring ReadOnlySpan<T> #9230

Merged
merged 5 commits into from
Jul 22, 2024

Conversation

h3xds1nz
Copy link
Contributor

@h3xds1nz h3xds1nz commented Jun 7, 2024

Description

Fixes unnecessary allocations on static readonly arrays by refactoring them as ReadOnlySpans<T> of primitive types.
This under the hood allows references in runtime to the data directly as stored in the mapped file, not requiring any runtime allocs. See RuntimeHelpers.CreateSpan<T> intrinsic for more info. In most cases the value is then inlined directly into the method, saving memory references as well.

In other cases, such as the Geometry classes, it allows us to reference data directly without any pinning (fixed is then merely used to retrieve the pointer that's passed to the unsafe method.

This also removes some unoptimized foreaches as replacement for IndexOf/Contains (guess that's coming from very early in the days).

In the end, it also removes inefficient foreach and IndexOf replacements for Contains methods with ROS<char>.Contains(value).

Method Mean Error StdDev Allocated
ReadOnlySpan_Contains 1.118 ns 0.0111 ns 0.0098 ns -
Original_Foreach_Contains 10.343 ns 0.0543 ns 0.0508 ns -

Customer Impact

Improved performance, decreased allocations.

Regression

None.

Testing

Build and standard use. Should be easy to CR.

Risk

Low.

Microsoft Reviewers: Open in CodeFlow

@h3xds1nz h3xds1nz requested a review from a team as a code owner June 7, 2024 16:23
@dotnet-policy-service dotnet-policy-service bot added PR metadata: Label to tag PRs, to facilitate with triage Community Contribution A label for all community Contributions labels Jun 7, 2024
@h3xds1nz h3xds1nz force-pushed the remove-static-arrays-allocs branch from f684b2b to 3fc9475 Compare June 7, 2024 18:23
@h3xds1nz
Copy link
Contributor Author

h3xds1nz commented Jun 8, 2024

I had to remove the optimization in PresentationBuildTasks as it still targets .NET Framework 4.7.2 and the build then fails.

@singhashish-wpf
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@anjali-wpf anjali-wpf left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution.

@anjali-wpf anjali-wpf merged commit 557ee98 into dotnet:main Jul 22, 2024
8 checks passed
@h3xds1nz
Copy link
Contributor Author

Thanks @anjali-wpf for including it.

@github-actions github-actions bot locked and limited conversation to collaborators Aug 22, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
Community Contribution A label for all community Contributions PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants