Skip to content

Use more spans in System.Reflection.Metadata et. al. - #76574

Merged
stephentoub merged 16 commits into
dotnet:mainfrom
teo-tsirpanis:srm-opt
Nov 3, 2022
Merged

Use more spans in System.Reflection.Metadata et. al.#76574
stephentoub merged 16 commits into
dotnet:mainfrom
teo-tsirpanis:srm-opt

Conversation

@teo-tsirpanis

Copy link
Copy Markdown
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.

@ghostghost added area-System.Reflection.Metadata community-contribution Indicates that the PR has been added by a community member labels Oct 3, 2022
@ghost

ghost commented Oct 3, 2022

Copy link
Copy Markdown

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:-

Copy link
Copy Markdown
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:

publicstaticvoidWriteUInt16(thisbyte[]buffer,intstart,ushortvalue)=>Unsafe.WriteUnaligned(refbuffer[start],BitConverter.IsLittleEndian?BinaryPrimitives.ReverseEndianness(value):value);

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done.

@steveharter

Copy link
Copy Markdown
Contributor

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

@steveharter

Copy link
Copy Markdown
Contributor

@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
Copy Markdown
ContributorAuthor

Done @steveharter.

@build-analysisbuild-analysisBot mentioned this pull request Oct 27, 2022
2 tasks

@stephentoubstephentoub left a comment

Copy link
Copy Markdown
Member

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:mainNov 3, 2022
@teo-tsirpanis
teo-tsirpanis deleted the srm-opt branch November 3, 2022 15:49
@ghostghost locked as resolved and limited conversation to collaborators Dec 3, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Reflection.Metadatacommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@teo-tsirpanis@steveharter@stephentoub@joperezr