SslStream-compatible ReadLine extension This code is a Extension to just be able to read a line quickly and efficiently with support for SslStream.

What i'm looking to achieve:


Lower CPU usage by improving how it stores bytes for being converted to string later
Lower CPU usage by possibly not reading per-byte some how. In my debug tests its using up the most CPU by far. 33%/55% of samples are the ReadByte


So your probably asking why im using a Queue as a buffer?
Basically the Queue allows me to more efficiently store bytes in an array format.
byte[] is un-acceptable due to the fact its size can't be changed dynamically (you would have to make a new byte, move over the old data, add the new, replace the old byte[], in-efficient)

queue however (or Stack if you .Reverse() it with linq) lets you add bytes whenever you want and then .ToArray when you need it as a byte[]
Same thing can be done with List&lt;byte&gt; but I dont feel like it's the best tool for the job in this case.

Another thing I could do here is just convert with Encoding.GetString on every byte and StringBuilder it but that seems to use more CPU and a lot more memory.

Any suggestions?

public static string ReadLine(this Stream stream, ref int bodySize, Encoding encoding) {
    bool bodySizeWasSpecified = bodySize &gt; 0;
    byte b = 0;
    Queue&lt;byte&gt; Buffer = new Queue&lt;byte&gt;();
    while (true) {
        #region Try Get 1 Byte from Stream
        try {
            int i = stream.ReadByte();
            if (i == -1) {
                break;//stream ended/closed
            }
            b = (byte)i;
        } catch {
            return null;//timeout //not authenticated context
        }
        #endregion
        #region If there's a body size specified, decrement back 1
        if (bodySizeWasSpecified) {
            bodySize--;
        }
        #endregion
        #region If Byte is \n or \r
        if (b == 10 || b == 13) {
            #region If ByteArray is Empty and the byte is \n reloop so we dont start with a leading \n
            if (Buffer.Count == 0 &amp;&amp; b == 10) {
                continue;
            }
            #endregion
            #region We hit a newline, lets finish the reads here.
            break;
            #endregion
        }
        #endregion
        #region Add the read byte to the Byte Array
        Buffer.Enqueue(b);
        #endregion
        #region Break if bodysize was greater than 0 but now its 0
        if (bodySizeWasSpecified &amp;&amp; bodySize == 0) {
            break;
        }
        #endregion
    }
    return encoding.GetString(Buffer.ToArray());
}
So your probably asking why im using a Queue as a buffer? Basically the Queue allows me to more efficiently store bytes in an array format. byte[] is un-acceptable due to the fact its size can't be changed dynamically (you would have to make a new byte, move over the old data, add the new, replace the old byte[], in-efficient)  


Well if you read the docs for Stream.ReadByte() you would see this 


  Notes to Inheritors
  
  The default implementation on Stream creates a new single-byte array and then calls Read(Byte[], Int32, Int32). While this is formally correct, it is inefficient. Any stream with an internal buffer should override this method and provide a much more efficient version that reads the buffer directly, avoiding the extra array allocation on every call.  


and if we dig into the reference source of Stream we would stumble over the default implementation  


public abstract int Read([In, Out] byte[] buffer, int offset, int count);

// Reads one byte from the stream by calling Read(byte[], int, int). 
// Will return an unsigned byte cast to an int or -1 on end of stream.
// This implementation does not perform well because it allocates a new
// byte[] each time you call it, and should be overridden by any 
// subclass that maintains an internal buffer.  Then, it can help perf
// significantly for people who are reading one byte at a time.
public virtual int ReadByte()
{
    Contract.Ensures(Contract.Result&lt;int&gt;() &gt;= -1);
    Contract.Ensures(Contract.Result&lt;int&gt;() &lt; 256);

    byte[] oneByteArray = new byte[1];
    int r = Read(oneByteArray, 0, 1);
    if (r==0)
        return -1;
    return oneByteArray[0];
}  



which isn't overwritten neither by NetworkStream nor by AuthenticatedStream (which is inherited by SslStream).

That beeing said, let's dig into the code...  

Region's are considered as an antipattern especially if they occur inside a method. Take a look at Are #regions an antipattern or code smell?  

Take e.g 

    #region If Byte is \n or \r
    if (b == 10 || b == 13)
    {  


this should be replaced by either  

    if (b == '\n' || b == '\r')
    {  


or by  

    const char NewLine = '\n';
    const char Return = '\r';

    if (b == NewLine || b == Return)
    {


I don't really like the catch here because it isn't specific. You should consider to remove the try..catch completely and let the calling code handle any thrown exception. This leads to another problem: The method is public but you don't validate if stream is null. You will just let the exception be thrown and catched by the catch I mentioned.  



I would add two overloaded ReadLine() methods to distinguish between needing the bodySize and let these methods return an IEnumerable&lt;byte&gt; which I would then convert to a byte[]. The methods would be called like so  

 public static string ReadLine(this Stream stream, ref int bodySize, Encoding encoding)
{
    if (stream == null) { throw new ArgumentNullException("stream"); }

    byte[] result;

    if (bodySize &gt; 0)
    {
        result = stream.ReadLine(bodySize).ToArray();
        bodySize -= result.Length;
    }
    else
    {
        result = stream.ReadLine().ToArray();
    }

    return encoding.GetString(result);

}


which is much easier to read.  

The methods themself could look like so  

private const char NewLine = '\n';
private const char Return = '\r';

private static IEnumerable&lt;byte&gt; ReadLine(this Stream stream, int bodySize)
{
    int i;
    int counter = 0;
    while (bodySize &gt; 0 &amp;&amp; (i = stream.ReadByte()) &gt; 0)
    {
        byte b = (byte)i;
        if (b == NewLine || b == Return)
        {
            // Break only if it isn't the first byte.
            if (counter &gt; 0 || b != NewLine) { yield break; }

            continue;

        }
        counter++;
        bodySize--;
        yield return b;
    }
}

private static IEnumerable&lt;byte&gt; ReadLine(this Stream stream)
{
    int i;
    int counter = 0;
    while ((i = stream.ReadByte()) &gt; 0)
    {
        byte b = (byte)i;
        if (b == NewLine || b == Return)
        {
            // Break only if it isn't the first byte.
            if (counter &gt; 0 || b != NewLine) { yield break; }

            continue;

        }
        counter++;
        yield return b;
    }
}  


or if we want to be clever we could change the second method like so  

private static IEnumerable&lt;byte&gt; ReadLine(this Stream stream)
{
    return stream.ReadLine(int.MaxValue);
}


Please note that the provided code isn't tested.