Compress SafeSocketHandle.Unix booleans

[antonfirsov]

There are 7 (+1 on Mac) bool fields in SafeSocketHandle.Unix which can be collapsed into a single byte field, saving 6 bytes per socket instance. The extra cost of the flag read/writes should be negligible.

[dotnet-policy-service]

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

[stephentoub]

saving 6 bytes per socket instance

Does it actually? If the fields were otherwise of a size divisible by eight, there's no difference on 64-bit between an extra byte field and an extra 7 boolean fields; the size of the object will still be rounded up to the next 8 byte boundary.

Unless there's a real savings, I'd rather keep the code simple.

[antonfirsov]

@EgorBot -intel

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System.Net;
using System.Net.Sockets;

BenchmarkSwitcher.FromAssembly(typeof(SocketBench).Assembly).Run(args);

[MemoryDiagnoser]
public class SocketBench
{
    private Socket? _listener;
    private Socket? _receiver;
    private Socket? _sender;
    private CancellationTokenSource? _cts;
    private byte[] _sendBuffer = new byte[16];

    [Benchmark]
    public int Send() => _sender!.Send(_sendBuffer);

    [Benchmark]
    public async Task<int> SendAsync() => await _sender!.SendAsync(_sendBuffer, SocketFlags.None, default);

    [Benchmark]
    public void CreateClose()
    {
        Socket sock = new Socket(SocketType.Stream, ProtocolType.Tcp);
        sock.Dispose();
    }

    [GlobalSetup(Targets = [nameof(Send), nameof(SendAsync)])]
    public async Task Setup()
    {
        _listener = new Socket(SocketType.Stream, ProtocolType.Tcp);
        _listener.Bind(new IPEndPoint(IPAddress.IPv6Loopback, 0));
        IPEndPoint serverEp = (IPEndPoint)_listener.LocalEndPoint!;
        _listener.Listen();

        Task<Socket> acceptTask = _listener.AcceptAsync();

        _sender = new Socket(SocketType.Stream, ProtocolType.Tcp);
        await _sender.ConnectAsync(serverEp);

        _receiver = await acceptTask;
        _cts = new CancellationTokenSource();

        _ = RunServer();

        async Task RunServer()
        {
            byte[] rcvBuffer = new byte[16];
            try
            {
                while (!_cts.Token.IsCancellationRequested)
                {
                    await _receiver.ReceiveAsync(rcvBuffer, _cts.Token);
                }
            }
            catch (OperationCanceledException) { }
        }
    }

    [GlobalCleanup(Targets = [nameof(Send), nameof(SendAsync)])]
    public void Cleanup()
    {
        _cts?.Cancel();
        _receiver?.Dispose();
        _listener?.Dispose();
        _sender?.Dispose();
    }
}

[antonfirsov]

The socket allocation cost went down 176 B -> 168 B, so it scales back the type's footprint by an 8 byte step.

@stephentoub if you think this is not worth the added complexity, I'll close this.

[Copilot]

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

src/libraries/System.Net.Sockets/src/System/Net/Sockets/SafeSocketHandle.Unix.cs:144

• The setter for IsNonBlocking does not handle the transition from non-blocking to blocking correctly, which could lead to unexpected socket behavior.

set { if (value) _flags |= Flags.NonBlocking; else _flags &= ~Flags.NonBlocking; }

src/libraries/System.Net.Sockets/src/System/Net/Sockets/SafeSocketHandle.Unix.cs:187

• The change in IsDisconnected property behavior from a private setter to a flag check might lead to unintended side effects.

internal bool IsDisconnected => (_flags & Flags.IsDisconnected) != 0;

[stephentoub]

The socket allocation cost went down 176 B -> 168 B, so it scales back the type's footprint by an 8 byte step.

@stephentoub if you think this is not worth the added complexity, I'll close this.

Ok, thanks. Looking at the type's fields, removing those bools causes it to cross a threshold that lowers the number of words by 1.

I'm ok with it, if others are.

[liveans]

LGTM, thanks!

[liveans]

nit: These are reusing same pattern, perhaps you can write a simple helper for this purpose.

[liveans]

/azp run runtime-libraries-coreclr outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[antonfirsov]

/azp run runtime-libraries-coreclr outerloop

[azure-pipelines]

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

[antonfirsov]

/azp run runtime-libraries-coreclr outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[antonfirsov]

/ba-g CI failures are unrelated