From adca031caebc543f7336bba36d3666342e1a0a54 Mon Sep 17 00:00:00 2001 From: Tomas Weinfurt Date: Fri, 14 Feb 2020 15:11:40 -0800 Subject: [PATCH 1/4] improve TLS perf on macOS --- .../Security/Pal.OSX/SafeDeleteSslContext.cs | 76 +++++++++---------- 1 file changed, 36 insertions(+), 40 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs index 1db55ec02444ab..d9aa4ec695c6dc 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs @@ -14,11 +14,12 @@ namespace System.Net { internal sealed class SafeDeleteSslContext : SafeDeleteContext { + private const int InitialBufferSize = 4096 * 4; private SafeSslHandle _sslContext; private Interop.AppleCrypto.SSLReadFunc _readCallback; private Interop.AppleCrypto.SSLWriteFunc _writeCallback; - private Queue _fromConnection = new Queue(); - private Queue _toConnection = new Queue(); + private ArrayBuffer _inputBuffer = new ArrayBuffer(InitialBufferSize); + private ArrayBuffer _outputBuffer = new ArrayBuffer(InitialBufferSize); public SafeSslHandle SslContext => _sslContext; @@ -143,6 +144,12 @@ protected override void Dispose(bool disposing) { if (null != _sslContext) { + lock (_sslContext) + { + _inputBuffer.Dispose(); + _outputBuffer.Dispose(); + } + _sslContext.Dispose(); _sslContext = null; } @@ -153,17 +160,14 @@ protected override void Dispose(bool disposing) private unsafe int WriteToConnection(void* connection, byte* data, void** dataLength) { - ulong toWrite = (ulong)*dataLength; - byte* readFrom = data; + int toWrite = (int)((ulong)*dataLength); + var inputBuffer = new ReadOnlySpan(data, toWrite); - lock (_toConnection) + lock (_sslContext) { - while (toWrite > 0) - { - _toConnection.Enqueue(*readFrom); - readFrom++; - toWrite--; - } + _outputBuffer.EnsureAvailableSpace(toWrite); + inputBuffer.CopyTo(_outputBuffer.AvailableSpan); + _outputBuffer.Commit(toWrite); } // Since we can enqueue everything, no need to re-assign *dataLength. @@ -180,30 +184,24 @@ private unsafe int ReadFromConnection(void* connection, byte* data, void** dataL if (toRead == 0) { - return noErr; } uint transferred = 0; - lock (_fromConnection) + lock (_sslContext) { - - if (_fromConnection.Count == 0) + if (_inputBuffer.ActiveLength == 0) { - *dataLength = (void*)0; return errSSLWouldBlock; } - byte* writePos = data; + int limit = Math.Min((int)toRead, _inputBuffer.ActiveLength); - while (transferred < toRead && _fromConnection.Count > 0) - { - *writePos = _fromConnection.Dequeue(); - writePos++; - transferred++; - } + _inputBuffer.ActiveSpan.Slice(0, limit).CopyTo(new Span(data, limit)); + _inputBuffer.Discard(limit); + transferred = (uint)limit; } *dataLength = (void*)transferred; @@ -222,30 +220,30 @@ internal void Write(byte[] buf, int offset, int count) internal void Write(ReadOnlySpan buf) { - lock (_fromConnection) + lock (_sslContext) { - foreach (byte b in buf) - { - _fromConnection.Enqueue(b); - } + _inputBuffer.EnsureAvailableSpace(buf.Length); + buf.CopyTo(_inputBuffer.AvailableSpan); + _inputBuffer.Commit(buf.Length); } } - internal int BytesReadyForConnection => _toConnection.Count; + internal int BytesReadyForConnection => _outputBuffer.ActiveLength; internal byte[] ReadPendingWrites() { - lock (_toConnection) + + lock (_sslContext) { - if (_toConnection.Count == 0) + if (_outputBuffer.ActiveLength == 0) { return null; } - byte[] data = _toConnection.ToArray(); - _toConnection.Clear(); + byte[] buffer = _outputBuffer.ActiveSpan.ToArray(); + _outputBuffer.Discard(_outputBuffer.ActiveLength); - return data; + return buffer; } } @@ -256,14 +254,12 @@ internal int ReadPendingWrites(byte[] buf, int offset, int count) Debug.Assert(count >= 0); Debug.Assert(count <= buf.Length - offset); - lock (_toConnection) + lock (_sslContext) { - int limit = Math.Min(count, _toConnection.Count); + int limit = Math.Min(count, _outputBuffer.ActiveLength); - for (int i = 0; i < limit; i++) - { - buf[offset + i] = _toConnection.Dequeue(); - } + _outputBuffer.ActiveSpan.Slice(0, limit).CopyTo(new Span(buf, offset, limit)); + _outputBuffer.Discard(limit); return limit; } From 8f3f67c702a5210a9f8f943d42322388c90c85d0 Mon Sep 17 00:00:00 2001 From: Tomas Weinfurt Date: Tue, 18 Feb 2020 15:17:07 -0800 Subject: [PATCH 2/4] feedback from review --- .../System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs index d9aa4ec695c6dc..a7168addf2ce0a 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs @@ -14,7 +14,7 @@ namespace System.Net { internal sealed class SafeDeleteSslContext : SafeDeleteContext { - private const int InitialBufferSize = 4096 * 4; + private const int InitialBufferSize = 2048; private SafeSslHandle _sslContext; private Interop.AppleCrypto.SSLReadFunc _readCallback; private Interop.AppleCrypto.SSLWriteFunc _writeCallback; @@ -160,7 +160,10 @@ protected override void Dispose(bool disposing) private unsafe int WriteToConnection(void* connection, byte* data, void** dataLength) { - int toWrite = (int)((ulong)*dataLength); + ulong length = (ulong)*dataLength; + Debug.Assert(length <= int.MaxValue); + + int toWrite = (int)length; var inputBuffer = new ReadOnlySpan(data, toWrite); lock (_sslContext) From bd3a247422bb6b22c74dc8f7c160cc14b31ec019 Mon Sep 17 00:00:00 2001 From: Tomas Weinfurt Date: Thu, 27 Feb 2020 14:54:25 -0800 Subject: [PATCH 3/4] feedback from review --- .../System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs index a7168addf2ce0a..802a9c99f3851e 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs @@ -142,16 +142,16 @@ protected override void Dispose(bool disposing) { if (disposing) { - if (null != _sslContext) + SafeSslHandle sslContext = _sslContext; + if (null != sslContext) { - lock (_sslContext) + lock (sslContext) { _inputBuffer.Dispose(); _outputBuffer.Dispose(); } - _sslContext.Dispose(); - _sslContext = null; + sslContext.Dispose(); } } From d3f7b1e6bfc2132e4404717fb800678968c1ec81 Mon Sep 17 00:00:00 2001 From: Tomas Weinfurt Date: Wed, 8 Apr 2020 20:28:41 -0700 Subject: [PATCH 4/4] remove unnecesary locking --- .../Security/Pal.OSX/SafeDeleteSslContext.cs | 83 ++++++------------- .../System/Net/Security/SslStreamPal.OSX.cs | 30 ++----- 2 files changed, 31 insertions(+), 82 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs index a8f50643f29985..72189aa2428012 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.OSX/SafeDeleteSslContext.cs @@ -145,12 +145,8 @@ protected override void Dispose(bool disposing) SafeSslHandle sslContext = _sslContext; if (null != sslContext) { - lock (sslContext) - { - _inputBuffer.Dispose(); - _outputBuffer.Dispose(); - } - + _inputBuffer.Dispose(); + _outputBuffer.Dispose(); sslContext.Dispose(); } } @@ -166,12 +162,9 @@ private unsafe int WriteToConnection(void* connection, byte* data, void** dataLe int toWrite = (int)length; var inputBuffer = new ReadOnlySpan(data, toWrite); - lock (_sslContext) - { - _outputBuffer.EnsureAvailableSpace(toWrite); - inputBuffer.CopyTo(_outputBuffer.AvailableSpan); - _outputBuffer.Commit(toWrite); - } + _outputBuffer.EnsureAvailableSpace(toWrite); + inputBuffer.CopyTo(_outputBuffer.AvailableSpan); + _outputBuffer.Commit(toWrite); // Since we can enqueue everything, no need to re-assign *dataLength. const int noErr = 0; @@ -182,7 +175,6 @@ private unsafe int ReadFromConnection(void* connection, byte* data, void** dataL { const int noErr = 0; const int errSSLWouldBlock = -9803; - ulong toRead = (ulong)*dataLength; if (toRead == 0) @@ -192,62 +184,42 @@ private unsafe int ReadFromConnection(void* connection, byte* data, void** dataL uint transferred = 0; - lock (_sslContext) + if (_inputBuffer.ActiveLength == 0) { - if (_inputBuffer.ActiveLength == 0) - { - *dataLength = (void*)0; - return errSSLWouldBlock; - } + *dataLength = (void*)0; + return errSSLWouldBlock; + } - int limit = Math.Min((int)toRead, _inputBuffer.ActiveLength); + int limit = Math.Min((int)toRead, _inputBuffer.ActiveLength); - _inputBuffer.ActiveSpan.Slice(0, limit).CopyTo(new Span(data, limit)); - _inputBuffer.Discard(limit); - transferred = (uint)limit; - } + _inputBuffer.ActiveSpan.Slice(0, limit).CopyTo(new Span(data, limit)); + _inputBuffer.Discard(limit); + transferred = (uint)limit; *dataLength = (void*)transferred; return noErr; } - internal void Write(byte[] buf, int offset, int count) - { - Debug.Assert(buf != null); - Debug.Assert(offset >= 0); - Debug.Assert(count >= 0); - Debug.Assert(count <= buf.Length - offset); - - Write(buf.AsSpan(offset, count)); - } - internal void Write(ReadOnlySpan buf) { - lock (_sslContext) - { - _inputBuffer.EnsureAvailableSpace(buf.Length); - buf.CopyTo(_inputBuffer.AvailableSpan); - _inputBuffer.Commit(buf.Length); - } + _inputBuffer.EnsureAvailableSpace(buf.Length); + buf.CopyTo(_inputBuffer.AvailableSpan); + _inputBuffer.Commit(buf.Length); } internal int BytesReadyForConnection => _outputBuffer.ActiveLength; internal byte[]? ReadPendingWrites() { - - lock (_sslContext) + if (_outputBuffer.ActiveLength == 0) { - if (_outputBuffer.ActiveLength == 0) - { - return null; - } + return null; + } - byte[] buffer = _outputBuffer.ActiveSpan.ToArray(); - _outputBuffer.Discard(_outputBuffer.ActiveLength); + byte[] buffer = _outputBuffer.ActiveSpan.ToArray(); + _outputBuffer.Discard(_outputBuffer.ActiveLength); - return buffer; - } + return buffer; } internal int ReadPendingWrites(byte[] buf, int offset, int count) @@ -257,15 +229,12 @@ internal int ReadPendingWrites(byte[] buf, int offset, int count) Debug.Assert(count >= 0); Debug.Assert(count <= buf.Length - offset); - lock (_sslContext) - { - int limit = Math.Min(count, _outputBuffer.ActiveLength); + int limit = Math.Min(count, _outputBuffer.ActiveLength); - _outputBuffer.ActiveSpan.Slice(0, limit).CopyTo(new Span(buf, offset, limit)); - _outputBuffer.Discard(limit); + _outputBuffer.ActiveSpan.Slice(0, limit).CopyTo(new Span(buf, offset, limit)); + _outputBuffer.Discard(limit); - return limit; - } + return limit; } private static readonly SslProtocols[] s_orderedSslProtocols = new SslProtocols[5] diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs index 7c6d13ac22bd69..b72e549355fdcc 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs @@ -92,16 +92,11 @@ public static SecurityStatusPal EncryptMessage( MemoryHandle memHandle = input.Pin(); try { - PAL_TlsIo status; - - lock (sslHandle) - { - status = Interop.AppleCrypto.SslWrite( + PAL_TlsIo status = Interop.AppleCrypto.SslWrite( sslHandle, (byte*)memHandle.Pointer, input.Length, out int written); - } if (status < 0) { @@ -154,19 +149,13 @@ public static SecurityStatusPal DecryptMessage( SafeDeleteSslContext sslContext = (SafeDeleteSslContext)securityContext; SafeSslHandle sslHandle = sslContext.SslContext; - sslContext.Write(buffer, offset, count); + sslContext.Write(buffer.AsSpan(offset, count)); unsafe { fixed (byte* offsetInput = &buffer[offset]) { - int written; - PAL_TlsIo status; - - lock (sslHandle) - { - status = Interop.AppleCrypto.SslRead(sslHandle, offsetInput, count, out written); - } + PAL_TlsIo status = Interop.AppleCrypto.SslRead(sslHandle, offsetInput, count, out int written); if (status < 0) { @@ -266,12 +255,7 @@ private static SecurityStatusPal HandshakeInternal( } SafeSslHandle sslHandle = sslContext!.SslContext; - SecurityStatusPal status; - - lock (sslHandle) - { - status = PerformHandshake(sslHandle); - } + SecurityStatusPal status = PerformHandshake(sslHandle); outputBuffer = sslContext.ReadPendingWrites(); return status; @@ -329,12 +313,8 @@ public static SecurityStatusPal ApplyShutdownToken( { SafeDeleteSslContext sslContext = ((SafeDeleteSslContext)securityContext); SafeSslHandle sslHandle = sslContext.SslContext; - int osStatus; - lock (sslHandle) - { - osStatus = Interop.AppleCrypto.SslShutdown(sslHandle); - } + int osStatus = Interop.AppleCrypto.SslShutdown(sslHandle); if (osStatus == 0) {