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

Reduce allocation in NegotiateStream.Read/Write{Async} #37772

Merged
merged 3 commits into from
Jun 12, 2020

Conversation

stephentoub
Copy link
Member

  1. Each time we call Write{Async}, we're allocating a new byte[] to store the encoded message to be written. Instead, just save the byte[] and reuse it, only allocating a new one if the existing one isn't big enough.
  2. In both Read/Write{Async}, we call into an EncryptDecryptHelper (on Windows) that allocates a byte[][]. It never has more than three buffers. We can just stack allocate it.
  3. In my previous change to switch to async/await, I used a default interface method. But when such a method is invoked on a struct implementing the interface, the struct gets boxed. Just move that instance off and avoid several allocations per read.
Method Toolchain Mean Ratio Gen 0 Allocated
WriteRead netcore31 14.99 ms 1.00 93.7500 616001 B
WriteRead master 13.24 ms 0.88 31.2500 200001 B
WriteRead pr 12.82 ms 0.85 - 1 B
ReadWrite netcore31 34.49 ms 1.00 71.4286 760230 B
ReadWrite master 33.30 ms 0.97 62.5000 528230 B
ReadWrite pr 32.76 ms 0.95 - 336240 B
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System.IO;
using System.Net;
using System.Net.Security;
using System.Net.Sockets;
using System.Threading.Tasks;

[MemoryDiagnoser]
public class Program
{
    static void Main(string[] args) => BenchmarkSwitcher.FromAssemblies(new[] { typeof(Program).Assembly }).Run(args);

    private Stream _client, _server;

    [GlobalSetup]
    public void ReadWriteSetup()
    {
        using (var listener = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp))
        {
            listener.Bind(new IPEndPoint(IPAddress.Loopback, 0));
            listener.Listen(1);

            var client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
            client.Connect(listener.LocalEndPoint);
            var server = listener.Accept();
            var clientStream = new NegotiateStream(new NetworkStream(client, ownsSocket: true), leaveInnerStreamOpen: true);
            var serverStream = new NegotiateStream(new NetworkStream(server, ownsSocket: true), leaveInnerStreamOpen: true);

            Task ca = clientStream.AuthenticateAsClientAsync();
            Task sa = serverStream.AuthenticateAsServerAsync();
            Task.WaitAll(ca, sa);

            _client = clientStream;
            _server = serverStream;
        }
    }

    private byte[] _oneByteBuffer = new byte[1];

    [Benchmark]
    public async Task WriteRead()
    {
        for (int i = 0; i < 1_000; i++)
        {
            await _server.WriteAsync(_oneByteBuffer);
            await _client.ReadAsync(_oneByteBuffer);
        }
    }

    [Benchmark]
    public async Task ReadWrite()
    {
        for (int i = 0; i < 1_000; i++)
        {
            var r = _client.ReadAsync(_oneByteBuffer);
            await _server.WriteAsync(_oneByteBuffer);
            await r;
        }
    }
}

Hold onto and reuse our write buffer.
The default interface method on the interface results in boxing a struct that implements that interface on each call to the method.
@ghost
Copy link

ghost commented Jun 11, 2020

Tagging subscribers to this area: @dotnet/ncl
Notify danmosemsft if you want to be subscribed.

@stephentoub stephentoub added this to the 5.0 milestone Jun 11, 2020
@stephentoub stephentoub added the tenet-performance Performance related issue label Jun 11, 2020
@stephentoub
Copy link
Member Author

cc: @davidsh, @brianrob

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM.
This is essentially what we do in SslStream, right?

@stephentoub
Copy link
Member Author

Yup

@stephentoub stephentoub merged commit 1408697 into dotnet:master Jun 12, 2020
@stephentoub stephentoub deleted the nsreadwritealloc branch June 12, 2020 09:34
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants