Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Remove 6 IntPtr fields from SocketAsyncEventArgs on Windows#22170

Merged
stephentoub merged 6 commits into
dotnet:masterfrom
stephentoub:remove_saea_fields
Jul 14, 2017
Merged

Remove 6 IntPtr fields from SocketAsyncEventArgs on Windows#22170
stephentoub merged 6 commits into
dotnet:masterfrom
stephentoub:remove_saea_fields

Conversation

@stephentoub
Copy link
Copy Markdown
Member

In the Windows implementation of SocketAsyncEventArgs, we're currently storing in six IntPtr fields values that we can efficiently recreate at run time, and by removing the fields, we save 48 bytes per SocketAsyncEventArgs instance (on 64-bit). Unfortunately SocketAsyncEventArgs is currently quite large, so that's "only" 7%, but it's a good start.

cc: @geoffkizer, @davidsh, @CIPop

@CIPop
Copy link
Copy Markdown

CIPop commented Jul 12, 2017

@stephentoub any noticeable impact on run-time perf? (From the changes I would't think so but worth trying out the Perf tests.)

pMessage->addressLength = (uint)_socketAddress.Size;
pMessage->buffers = _ptrWSARecvMsgWSABufferArray;
if (_buffer != null)
fixed (void* ptrWSARecvMsgWSABufferArray = &wsaRecvMsgWSABufferArray[0])
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why is this better than UnsafeAddrOfPinnedArrayElement?

Copy link
Copy Markdown
Member Author

@stephentoub stephentoub Jul 12, 2017

Choose a reason for hiding this comment

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

It's a bit faster, at least in the tests I did. I tried this, UnsafeAddrOfPinnedArrayElement, GCHandle.AddrOfPinnedObject, etc., and this won so I went with it.

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.

UnsafeAddrOfPinnedArrayElement, GCHandle.AddrOfPinnedObject

All these methods have argument validation, etc. that makes them slower than necessary.

pMessage->buffers = (IntPtr)Unsafe.AsPointer(ref wsaRecvMsgWSABufferArray[0]) should be the fastest way to do this (a few instructions better than the dummy fixed statement), but it would require adding a dependency on S.R.CS.Unsafe.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks, Jan.

@karelz karelz modified the milestone: 2.1.0 Jul 14, 2017
@stephentoub
Copy link
Copy Markdown
Member Author

any noticeable impact on run-time perf?

No. For the most part the used fields aren't on code paths exercised by the perf tests, which are mainly focused on Connect/Accept/Send/Receive.

@stephentoub stephentoub merged commit 1fff38f into dotnet:master Jul 14, 2017
@stephentoub stephentoub deleted the remove_saea_fields branch July 14, 2017 21:37
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…ields

Remove 6 IntPtr fields from SocketAsyncEventArgs on Windows

Commit migrated from dotnet/corefx@1fff38f
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants