Skip to content

Refactor Span<T>.ToArray #119462

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
Open
xtqqczze wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Refactor Span<T>.ToArray #119462

xtqqczze wants to merge 1 commit into from

Conversation

@xtqqczze
Copy link
Contributor

@xtqqczze xtqqczze commented Sep 8, 2025

Refactor to reduce unsafe.

@xtqqczze
Refactor Span<T>.ToArray
bd97548 
Refactor to reduce unsafe.
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Sep 8, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Sep 8, 2025
tannergooding
tannergooding reviewed Sep 8, 2025
View reviewed changes
src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs
Buffer.Memmove(ref MemoryMarshal.GetArrayDataReference(destination), ref _reference, (uint)_length);
return destination;
T[] array = new T[Length];
CopyTo(array);
Copy link
Member

@tannergooding tannergooding Sep 8, 2025

Choose a reason for hiding this comment

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

nit: Only this line needs to be changed to reduce unsafe.

destination is arguably a clearer name and the use of _length == 0 and new T[_length] isn't at all unsafe, plus it reduces inlining needs for a known perf critical type so we only have one path that actually needs inlining considerations.

@xtqqczze
Copy link
Contributor Author

xtqqczze commented Sep 8, 2025

Why is superpmi-diffs check skipped?

@tannergooding
Copy link
Member

tannergooding commented Sep 8, 2025

Why is superpmi-diffs check skipped?

It's only run for JIT changes, not all library changes.

@xtqqczze
Copy link
Contributor Author

xtqqczze commented Sep 8, 2025

Why is superpmi-diffs check skipped?

It's only run for JIT changes, not all library changes.

How can I check for Windows codegen regressions?

@EgorBo
Copy link
Member

EgorBo commented Sep 8, 2025 •
edited
Loading

How can I check for Windows codegen regressions?

Run jit-diff locally? or at least provide benchmarks.
SPMI diffs can't catch changes in the libraries by design

@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Oct 6, 2025

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

@teo-tsirpanis teo-tsirpanis removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Oct 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tannergooding tannergooding tannergooding left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

area-System.Memory community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@xtqqczze @tannergooding @EgorBo @marek-safar @teo-tsirpanis