Use the read buffer in UploadFile for the SFTP write packets (sshnet#1798)

* Use the read buffer in UploadFile for the SFTP write packets

In SftpClient.UploadFile, a buffer is allocated to read from the given stream, and for
each read, another array is allocated for the SFTP write packet (which consists of that
data prepended with headers). This change effectively leaves space at the start of the
buffer for the headers such that it can be used to assemble the packets without that
per-packet array allocation.

There are cleaner/more general ways to do this (e.g. for all packet types, leave space
for the SSH headers as well), but this gets the most impact for about as much effort as
I can be bothered with.

* Rent from pool

Author: Rob-Hague

src/Renci.SshNet/Sftp/ISftpSession.cs

@@ -4,6 +4,7 @@
 using System.Threading.Tasks;
 
 using Renci.SshNet.Common;
+using Renci.SshNet.Sftp.Requests;
 using Renci.SshNet.Sftp.Responses;
 
 namespace Renci.SshNet.Sftp
@@ -325,14 +326,19 @@ internal interface ISftpSession : ISubsystemSession
         /// <param name="offset">the zero-based offset in <paramref name="data" /> at which to begin taking bytes to write.</param>
         /// <param name="length">The length (in bytes) of the data to write.</param>
         /// <param name="wait">The wait event handle if needed.</param>
-        /// <param name="writeCompleted">The callback to invoke when the write has completed.</param>
         void RequestWrite(byte[] handle,
                           ulong serverOffset,
                           byte[] data,
                           int offset,
                           int length,
-                          AutoResetEvent wait,
-                          Action<SftpStatusResponse> writeCompleted = null);
+                          AutoResetEvent wait);
+
+        /// <summary>
+        /// Performs SSH_FXP_WRITE request.
+        /// </summary>
+        /// <param name="buffer">The buffer.</param>
+        /// <param name="writeCompleted">The callback to invoke when the write has completed.</param>
+        void RequestWrite(SftpWriteRequestBuffer buffer, Action<SftpStatusResponse> writeCompleted);
 
         /// <summary>
         /// Asynchronouly performs a <c>SSH_FXP_WRITE</c> request.

src/Renci.SshNet/Sftp/Requests/SftpWriteRequest.cs

@@ -1,17 +1,27 @@
 using System;
+using System.Buffers.Binary;
 
+using Renci.SshNet.Common;
 using Renci.SshNet.Sftp.Responses;
 
 namespace Renci.SshNet.Sftp.Requests
 {
     internal sealed class SftpWriteRequest : SftpRequest
     {
+        private readonly SftpWriteRequestBuffer _buffer;
+
         public override SftpMessageTypes SftpMessageType
         {
             get { return SftpMessageTypes.Write; }
         }
 
-        public byte[] Handle { get; private set; }
+        public ReadOnlySpan<byte> Handle
+        {
+            get
+            {
+                return _buffer.Handle;
+            }
+        }
 
         /// <summary>
         /// Gets the zero-based offset (in bytes) relative to the beginning of the file that the write
@@ -21,82 +31,69 @@ public override SftpMessageTypes SftpMessageType
         /// The zero-based offset (in bytes) relative to the beginning of the file that the write must
         /// start at.
         /// </value>
-        public ulong ServerFileOffset { get; private set; }
+        public ulong ServerFileOffset
+        {
+            get
+            {
+                return _buffer.ServerFileOffset;
+            }
+        }
 
         /// <summary>
         /// Gets the buffer holding the data to write.
         /// </summary>
         /// <value>
         /// The buffer holding the data to write.
         /// </value>
-        public byte[] Data { get; private set; }
-
-        /// <summary>
-        /// Gets the zero-based offset in <see cref="Data" /> at which to begin taking bytes to
-        /// write.
-        /// </summary>
-        /// <value>
-        /// The zero-based offset in <see cref="Data" /> at which to begin taking bytes to write.
-        /// </value>
-        public int Offset { get; private set; }
-
-        /// <summary>
-        /// Gets the length (in bytes) of the data to write.
-        /// </summary>
-        /// <value>
-        /// The length (in bytes) of the data to write.
-        /// </value>
-        public int Length { get; private set; }
+        public ReadOnlySpan<byte> Data
+        {
+            get
+            {
+                return _buffer.Data.AsSpan(0, _buffer.DataLength);
+            }
+        }
 
         protected override int BufferCapacity
         {
             get
             {
-                var capacity = base.BufferCapacity;
-                capacity += 4; // Handle length
-                capacity += Handle.Length; // Handle
-                capacity += 8; // ServerFileOffset length
-                capacity += 4; // Data length
-                capacity += Length; // Data
-                return capacity;
+                return _buffer.ActiveBytes.Count;
             }
         }
 
         public SftpWriteRequest(uint protocolVersion,
-                                uint requestId,
-                                byte[] handle,
-                                ulong serverFileOffset,
-                                byte[] data,
-                                int offset,
-                                int length,
+                                SftpWriteRequestBuffer buffer,
                                 Action<SftpStatusResponse> statusAction)
-            : base(protocolVersion, requestId, statusAction)
+            : base(protocolVersion, buffer.RequestId, statusAction)
         {
-            Handle = handle;
-            ServerFileOffset = serverFileOffset;
-            Data = data;
-            Offset = offset;
-            Length = length;
+            _buffer = buffer;
         }
 
         protected override void LoadData()
         {
-            base.LoadData();
-
-            Handle = ReadBinary();
-            ServerFileOffset = ReadUInt64();
-            Data = ReadBinary();
-            Offset = 0;
-            Length = Data.Length;
+            throw new NotImplementedException();
         }
 
         protected override void SaveData()
         {
-            base.SaveData();
+            throw new NotImplementedException();
+        }
+
+        protected override void WriteBytes(SshDataStream stream)
+        {
+            var activeBuffer = GetBytes();
+
+            stream.Write(activeBuffer.Array, activeBuffer.Offset, activeBuffer.Count);
+        }
+
+        public new ArraySegment<byte> GetBytes()
+        {
+            var activeBuffer = _buffer.ActiveBytes;
+
+            // Write SFTP packet length.
+            BinaryPrimitives.WriteInt32BigEndian(activeBuffer.AsSpan(), activeBuffer.Count - 4);
 
-            WriteBinaryString(Handle);
-            Write(ServerFileOffset);
-            WriteBinary(Data, Offset, Length);
+            return activeBuffer;
         }
     }
 }

src/Renci.SshNet/Sftp/Requests/SftpWriteRequestBuffer.cs

@@ -0,0 +1,149 @@
+#nullable enable
+using System;
+using System.Buffers;
+using System.Buffers.Binary;
+using System.Diagnostics;
+
+namespace Renci.SshNet.Sftp.Requests
+{
+    /// <summary>
+    /// A helper type that wraps a buffer for SFTP write requests.
+    /// </summary>
+    /// <remarks>
+    /// [Sftp packet length, SftpMessageType, RequestId, Handle length, Handle, Server offset, data length, data].
+    /// [                 4,               1,         4,             4,      ?,             8,           4,    ?].
+    /// </remarks>
+    internal sealed class SftpWriteRequestBuffer : IDisposable
+    {
+        private const int MessageTypeOffset = 4;
+        private const int RequestIdOffset = MessageTypeOffset + 1;
+        private const int HandleLengthOffset = RequestIdOffset + 4;
+        private const int HandleOffset = HandleLengthOffset + 4;
+
+        private readonly bool _usePool;
+        private byte[] _buffer;
+
+        public ArraySegment<byte> ActiveBytes
+        {
+            get
+            {
+                return new(_buffer, 0, HandleOffset + HandleLength + 8 + 4 + DataLength);
+            }
+        }
+
+        public SftpWriteRequestBuffer(ReadOnlySpan<byte> handle, int dataCapacity, bool usePool = false)
+        {
+            Debug.Assert(dataCapacity >= 0);
+
+            var totalCapacity = HandleOffset + handle.Length + 8 + 4 + dataCapacity;
+
+            _usePool = usePool;
+
+            _buffer = usePool
+                ? ArrayPool<byte>.Shared.Rent(totalCapacity)
+                : new byte[totalCapacity];
+
+            _buffer[MessageTypeOffset] = (byte)SftpMessageTypes.Write;
+
+            HandleLength = handle.Length;
+
+            handle.CopyTo(_buffer.AsSpan(HandleOffset));
+        }
+
+        public SftpWriteRequestBuffer(ReadOnlySpan<byte> handle, ulong serverFileOffset, ReadOnlySpan<byte> data, bool usePool = false)
+            : this(handle, data.Length, usePool)
+        {
+            ServerFileOffset = serverFileOffset;
+
+            DataLength = data.Length;
+
+            data.CopyTo(Data);
+        }
+
+        public uint RequestId
+        {
+            get
+            {
+                return BinaryPrimitives.ReadUInt32BigEndian(_buffer.AsSpan(RequestIdOffset));
+            }
+            set
+            {
+                BinaryPrimitives.WriteUInt32BigEndian(_buffer.AsSpan(RequestIdOffset), value);
+            }
+        }
+
+        public int HandleLength
+        {
+            get
+            {
+                return BinaryPrimitives.ReadInt32BigEndian(_buffer.AsSpan(HandleLengthOffset));
+            }
+            private init
+            {
+                Debug.Assert(value >= 0);
+                BinaryPrimitives.WriteInt32BigEndian(_buffer.AsSpan(HandleLengthOffset), value);
+            }
+        }
+
+        public ReadOnlySpan<byte> Handle
+        {
+            get
+            {
+                return _buffer.AsSpan(HandleOffset, HandleLength);
+            }
+        }
+
+        public ulong ServerFileOffset
+        {
+            get
+            {
+                return BinaryPrimitives.ReadUInt64BigEndian(_buffer.AsSpan(HandleOffset + HandleLength));
+            }
+            set
+            {
+                BinaryPrimitives.WriteUInt64BigEndian(_buffer.AsSpan(HandleOffset + HandleLength), value);
+            }
+        }
+
+        public int DataLength
+        {
+            get
+            {
+                return BinaryPrimitives.ReadInt32BigEndian(_buffer.AsSpan(HandleOffset + HandleLength + 8));
+            }
+            set
+            {
+                Debug.Assert(value >= 0);
+                Debug.Assert(value <= _buffer.Length - (HandleOffset + HandleLength + 8 + 4));
+
+                BinaryPrimitives.WriteInt32BigEndian(_buffer.AsSpan(HandleOffset + HandleLength + 8), value);
+            }
+        }
+
+        /// <summary>
+        /// Gets the space available to write as file data. Does not consider <see cref="DataLength"/>.
+        /// </summary>
+        public ArraySegment<byte> Data
+        {
+            get
+            {
+                var offset = HandleOffset + HandleLength + 8 + 4;
+                return new ArraySegment<byte>(_buffer, offset, _buffer.Length - offset);
+            }
+        }
+
+        public void Dispose()
+        {
+            if (_usePool)
+            {
+                var buffer = _buffer;
+                _buffer = null!;
+
+                if (buffer is not null)
+                {
+                    ArrayPool<byte>.Shared.Return(buffer);
+                }
+            }
+        }
+    }
+}