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

[API Proposal]: Efficient EndPoint serialization #78993

Open
antonfirsov opened this issue Nov 29, 2022 · 6 comments
Open

[API Proposal]: Efficient EndPoint serialization #78993

antonfirsov opened this issue Nov 29, 2022 · 6 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net.Sockets
Milestone

Comments

@antonfirsov
Copy link
Member

antonfirsov commented Nov 29, 2022

Background and motivation

Serializing general EndPoint instances into buffers/structures to be used with low-level interop code requires instantiating a SocketAddress and copying its bytes to a separate buffer manually. This leads to 2 extra allocations: SocketAddress and the underlying buffer. This is one of the main reasons our SendTo/ReceiveFrom methods allocate a lot, see discussion in #30797.

We can workaround this by special-casing IPEndPoint (#78591), but that would introduce complexity to the Socket codebase if implemented across all sync and async variants and doesn't work with arbitrary endpoints. We can also expose SocketAddress.Buffer (#78757), but IMO it's a partial solution, since it doesn't help with the allocations.

Instead of working around our own inefficient abstractions, we should consider adding an alternative modern and efficient API to serialize arbitrary EndPoint-s into user-provided buffers. This would help simplifying and improving socket code while keeping it compatible with arbitrary endpoints.

API Proposal

namespace System.Net;

public abstract class EndPoint
{
    // Returns true if the EndPoint is serializable into a linear native sockaddr_xy structure
    public virtual bool TryGetSocketAddressSize(out int size);

    // Existing:
    // public virtual SocketAddress Serialize();
    
    // Proposed:
    public virtual void Serialize(Span<byte> socketAddress);
    
    // Existing:
    // public virtual EndPoint Create(SocketAddress socketAddress);
    
    // Proposed:
    public virtual EndPoint Create(ReadOnlySpan<byte> socketAddress);
}

API Usage

if (endPoint.TryGetSocketAddressSize(out int size))
{
    Span<byte> socketAddress = size < 256 ? stackalloc byte[size] : new byte[size];
    endPoint.Serialize(socketAddress);
    
    // use socketAddress
}

Alternative Designs

I can't think of a significantly different design that would also delegate the buffer lifecycle management to the consumer of the API.

If we think that DnsEndpoint is the only exception that is not serializable and needs special threatment, we can expose an int Size property instead of TryGetSocketAddressSize.

Risks

Considering compatibility, the only correct way to implement TryGetSocketAddressSize(out int size) is inefficient and may regress path that uses existing 3rd party endpoints which are not updated with overloads:

public virtual bool TryGetSocketAddressSize(out int size)
{
    try
    {
        size = this.Serialize().Size;
        return true;
    }
    catch
    {
        size = default;
        return false;
    }
}

This is not a concern if we think DnsEndPoint is the only special case that is not serializable.

@antonfirsov antonfirsov added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net.Sockets labels Nov 29, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 29, 2022
@ghost
Copy link

ghost commented Nov 29, 2022

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

Issue Details

Background and motivation

Serializing general EndPoint instances into buffers/structures to be used with low-level interop code requires instantiating a SocketAddress and copying its bytes to a separate buffer manually. This leads to 2 extra allocations: SocketAddress and the underlying buffer. This is one of the main reasons our SendTo/ReceiveFrom methods allocate a lot, see discussion in #30797.

We can workaround this by special-casing IPEndPoint (#78591), but that would introduce complexity to the Socket codebase if implemented across all sync and async variants and doesn't work with arbitrary endpoints. We can also expose SocketAddress.Buffer (#78757), but IMO it's a partial solution, since it doesn't help with the allocations.

Instead working around our own inefficient abstractions, we should consider adding an alternative modern and efficient API to serialize arbitrary EndPoint-s into user-provided buffers. This would help simplifying and improving socket code while keeping it compatible with arbitrary endpoints.

API Proposal

namespace System.Net;

public abstract class EndPoint
{
    // Existing:
    // public virtual SocketAddress Serialize();
    // public virtual EndPoint Create(SocketAddress socketAddress);

    // Returns true if the EndPoint is serializable into a linear native structure
    public virtual bool TryGetSocketAddressSize(out int size);
    public virtual void Serialize(Span<byte> socketAddress);
    public virtual EndPoint Create(ReadOnlySpan<byte> socketAddress);
}

API Usage

if (endPoint.TryGetSocketAddressSize(out int size))
{
    Span<byte> socketAddress = size < 256 ? stackalloc byte[size] : new byte[size];
    endPoint.Serialize(socketAddress);
    
    // use socketAddress
}

Alternative Designs

I can't think of a significantly different design that would also delegate the buffer lifecycle management to the user.

Risks

No response

Author: antonfirsov
Assignees: -
Labels:

api-suggestion, area-System.Net.Sockets

Milestone: -

@fbrosseau
Copy link

What would the default Create(ROS) implementation be to keep backward compatibility? A call to Create(SocketAddress)?

@antonfirsov
Copy link
Member Author

Yes, that would be the only way to keep things compatible. TryGetSocketAddressSize(out int) is more tricky, I updated the Risks section.

@ericstj
Copy link
Member

ericstj commented Dec 7, 2022

Seems odd that Create looks like a factory method but it's not static. Maybe add sample API usage that shows how that's used?

@antonfirsov
Copy link
Member Author

antonfirsov commented Dec 7, 2022

@ericstj it's matching the existing overload. The proposed overload takes ReadOnlySpan<byte> instead of a SocketAddress.

The usage is unusual, but it's legacy design: an existing EndPoint instance is used to create a new instance of the same subclass from native socket address buffer, see Socket.cs. It meant to be invoked by low-level networking implementations, while third parties can implement EndPoints of arbitrary types by overriding Create and Serialize.

@antonfirsov
Copy link
Member Author

Triage: depends on the fate of #30797, moving to future.

@antonfirsov antonfirsov removed the untriaged New issue has not been triaged by the area owner label Feb 2, 2023
@antonfirsov antonfirsov added this to the Future milestone Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net.Sockets
Projects
None yet
Development

No branches or pull requests

3 participants