Skip to content

Comments

Remove RawArrayData#110902

Closed
MichalPetryka wants to merge 3 commits intodotnet:mainfrom
MichalPetryka:array-layout
Closed

Remove RawArrayData#110902
MichalPetryka wants to merge 3 commits intodotnet:mainfrom
MichalPetryka:array-layout

Conversation

@MichalPetryka
Copy link
Contributor

Removes RawArrayData and makes Array reflect the actual runtime layout instead.

Makes code a bit safer by removing Unsafe.As usages with invalid types.

cc @EgorBo @tannergooding I ended up doing this after the discussion on Discord
cc @jkotas There was one CoreCLR assert here due to array MT creation not setting the field count, I made it set it here in debug to avoid that and any perf impact in release, is that okay to do?

@ghost ghost added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Dec 23, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Dec 23, 2024
@MichalPetryka
Copy link
Contributor Author

@MihuBot

// This field should be the first field in Array as the runtime/compilers depend on it
private int _numComponents;
#pragma warning restore
internal uint RawLength; // Array._numComponents padded to IntPtr
Copy link
Member

Choose a reason for hiding this comment

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

This should follow dotnet/runtime naming conventions for fields.

@jkotas
Copy link
Member

jkotas commented Dec 23, 2024

@jkotas There was one CoreCLR assert here due to array MT creation not setting the field count, I made it set it here in debug to avoid that and any perf impact in release, is that okay to do?

I do not see a problem with this performance wise. However, I expect that there is going to be a long tail of places that are not prepared to handle fields on array type. Some of them may be outside dotnet/runtime. There may be more straightforward (less breaking) ways to get rid of RawArrayData. For example, call object.GetRawData() and cast it to the array layout? Or introduce JIT intrinsics for raw length and raw array data getters.

@dotnet-policy-service
Copy link
Contributor

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

@mangod9 mangod9 added the needs-author-action An issue or pull request that requires more info or actions from the author. label Feb 3, 2025
@dotnet-policy-service
Copy link
Contributor

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

@dotnet-policy-service
Copy link
Contributor

This pull request will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the pull request, but please note that it will be locked if it remains inactive for another 30 days.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 3, 2025
@am11 am11 removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Apr 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-VM-coreclr community-contribution Indicates that the PR has been added by a community member needs-author-action An issue or pull request that requires more info or actions from the author. no-recent-activity

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants