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 more spans in System.Reflection.Metadata et. al. #76574

Merged
merged 16 commits into from
Nov 3, 2022

Conversation

teo-tsirpanis
Copy link
Contributor

This PR among other things changes the buffer code of System.Reflection.Metadata so that it uses spans and framework methods that are more optimized, and reduces pinning and unsafely converting between immutable and mutable arrays.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Oct 3, 2022
@ghost
Copy link

ghost commented Oct 3, 2022

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

Issue Details

This PR among other things changes the buffer code of System.Reflection.Metadata so that it uses spans and framework methods that are more optimized, and reduces pinning and unsafely converting between immutable and mutable arrays.

Author: teo-tsirpanis
Assignees: -
Labels:

area-System.Reflection.Metadata

Milestone: -

}
Unsafe.WriteUnaligned(ref buffer[start], value);
Copy link
Member

Choose a reason for hiding this comment

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

Nit: You could tidy this (and the others like it) up to just be:

public static void WriteUInt16(this byte[] buffer, int start, ushort value) =>
    Unsafe.WriteUnaligned(ref buffer[start], BitConverter.IsLittleEndian ? BinaryPrimitives.ReverseEndianness(value) : value);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@steveharter
Copy link
Member

Are there any remaining CI issues? There were some Mono build failures, but I suspect infrastructure and just re-ran those.

@steveharter
Copy link
Member

@teo-tsirpanis can you rebase to the latest? There are these build errors that have already recently been fixed (see this PR):

D:\a\_work\1\s\src\mono\dlls\mscordbi\cordb-value.cpp(351): error C2259: 'CordbArrayValue': cannot instantiate abstract class
2022-10-26T18:05:27.8469947Z   D:\a\_work\1\s\src\mono\dlls\mscordbi\cordb-value.h(170): note: see declaration of 'CordbArrayValue'
2022-10-26T18:05:27.8905158Z   D:\a\_work\1\s\src\mono\dlls\mscordbi\cordb-value.cpp(351): note: due to following members:

@teo-tsirpanis
Copy link
Contributor Author

Done @steveharter.

@build-analysis build-analysis bot mentioned this pull request Oct 27, 2022
2 tasks
Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

Thanks.

@stephentoub stephentoub merged commit b8a0906 into dotnet:main Nov 3, 2022
@teo-tsirpanis teo-tsirpanis deleted the srm-opt branch November 3, 2022 15:49
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Reflection.Metadata community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants