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

Perf | Reduce memory allocations in SerializeEncodingChar/WriteEncodingChar and some options boxing #785

Merged
merged 3 commits into from
Feb 24, 2021

Conversation

coderb
Copy link
Contributor

@coderb coderb commented Nov 6, 2020

these changes reduce unnecessary memory allocations which results in decreased memory pressure on the garbage collector and better performance. the changes are all localized so hopefully are easy to review and accept.

@dnfadmin
Copy link

dnfadmin commented Nov 6, 2020

CLA assistant check
All CLA requirements met.

@coderb
Copy link
Contributor Author

coderb commented Nov 6, 2020

The test that is failing seems to be unrelated to my change:
Microsoft.Data.SqlClient.Tests.SqlErrorCollectionTest.CopyTo_Throws [1s]

@Wraith2
Copy link
Contributor

Wraith2 commented Nov 10, 2020

The HasFlags changes aren't needed and I put them in because I was advised to by the jit team because HasFlags is a jit intrinsic on netcore2.1 and later and gets optimized out. sharplab example compare the il and asm in release and you'll see the box is totally removed. If you're using netfx then you've got a lot bigger problems with memory than these calls.

@coderb
Copy link
Contributor Author

coderb commented Nov 10, 2020

@Wraith2 ok, i've reverted that change. is there a blog post or documentation on the jit optimization/intrinsic you are referring to? (n/m found a writeup). ... i don't see the advantage of HasFlag over the bit test (which never allocates). but ok.

@Wraith2
Copy link
Contributor

Wraith2 commented Nov 10, 2020

It's covered in this blog post https://devblogs.microsoft.com/dotnet/performance-improvements-in-net-core-2-1/ but I found it while reviewing PR's in the runtime repo where it was suggested to use HasFlag instead of the masking. The advantage is that the jit sees the call and has a direct transformation for it instead of relying on the translation and simplification steps that should result in the same codegen but might not.

@cheenamalhotra cheenamalhotra added this to the 3.0.0-preview1 milestone Feb 23, 2021
@cheenamalhotra cheenamalhotra merged commit c92b2ca into dotnet:master Feb 24, 2021
@cheenamalhotra cheenamalhotra changed the title Memperf: reduce allocations in SerializeEncodingChar/WriteEncodingChar and some options boxing Perf | Reduce memory allocations in SerializeEncodingChar/WriteEncodingChar and some options boxing Feb 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants