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

Read/Write Big/LittleEndian support for Guid #87993

Merged

Conversation

AlexRadch
Copy link
Contributor

Close #86798 issue.

@dotnet-issue-labeler dotnet-issue-labeler bot added needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners new-api-needs-documentation labels Jun 24, 2023
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Jun 24, 2023
@dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@teo-tsirpanis teo-tsirpanis added area-System.Runtime and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jun 24, 2023
@ghost
Copy link

ghost commented Jun 24, 2023

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

Issue Details

Close #86798 issue.

Author: AlexRadch
Assignees: -
Labels:

area-System.Runtime, new-api-needs-documentation, community-contribution

Milestone: -

MemoryMarshal.Read<Guid>(b);
[DoesNotReturn]
@tannergooding
Copy link
Member

@AlexRadch, looks like this was just pending some basic perf numbers. Were you able to collect those?

@AlexRadch
Copy link
Contributor Author

@AlexRadch, looks like this was just pending some basic perf numbers. Were you able to collect those?

I already wrote. I got next scores:

|               Type |           Method |       Mean |     Error |    StdDev | Ratio | RatioSD |   Gen0 | Allocated | Alloc Ratio |
|------------------- |----------------- |-----------:|----------:|----------:|------:|--------:|-------:|----------:|------------:|
|      BenchFromSpan |      GO_FromSpan |  58.594 ns | 0.4230 ns | 0.3750 ns |  1.00 |    0.00 |      - |         - |          NA |
|      BenchFromSpan |      GN_FromSpan |   6.271 ns | 0.1132 ns | 0.0884 ns |  0.11 |    0.00 |      - |         - |          NA |
|   BenchToByteArray |   GO_ToByteArray | 168.241 ns | 1.3852 ns | 1.2957 ns |  2.87 |    0.03 | 0.0956 |     400 B |          NA |
|   BenchToByteArray |   GN_ToByteArray | 139.170 ns | 1.2337 ns | 1.1540 ns |  2.38 |    0.02 | 0.0956 |     400 B |          NA |
| BenchTryWriteBytes | GO_TryWriteBytes |  70.844 ns | 1.4296 ns | 1.3372 ns |  1.21 |    0.02 |      - |         - |          NA |
| BenchTryWriteBytes | GN_TryWriteBytes |  14.141 ns | 0.0899 ns | 0.0841 ns |  0.24 |    0.00 |      - |         - |          NA |

Tests are only for slow paths and only for little-endian processors.

New constructor is 10 times faster on my computer.
New ToByteArray is a little faster on my computer.
New TryWriteBytes is 5 times faster on my computer.

Source code https://github.com/AlexRadch/Temp/tree/Guid_BigLittleEndian_Bench

@tannergooding
Copy link
Member

Thanks! I didn't see those results anywhere in the comments for this PR when I looked. I must have missed them.

@tannergooding tannergooding merged commit e70f0df into dotnet:main Jul 11, 2023
@AlexRadch AlexRadch deleted the ReadWrite_BigLittle_Endian_ForGuid branch July 11, 2023 20:55
@ghost ghost locked as resolved and limited conversation to collaborators Aug 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime community-contribution Indicates that the PR has been added by a community member new-api-needs-documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Read/Write Big/LittleEndian support for Guid
7 participants