Improving the performance of a BinaryReader

Question

I am currently in the process of writing a BinaryReader that caches the BaseStream.Position and BaseStream.Length properties. Here is what I have so far:

public class FastBinaryReader
{
    BinaryReader reader;

    public long Length { get; private set; }
    public long Position { get; private set; }

    public FastBinaryReader(Stream stream)
    {
        reader = new BinaryReader(stream);
        Length = stream.Length;
        Position = 0;
    }

    public void Seek(long newPosition)
    {
        reader.BaseStream.Position = newPosition;
        Position = newPosition;
    }

    public byte[] ReadBytes(int count)
    {
        if (Position + count >= Length)
            Position = Length;
        else
            Position += count;

        return reader.ReadBytes(count);
    }

    public void Close()
    {
        reader.Close();
    }
}

Instead of providing a Length and Position property, I would like to create a BaseStream property that allows me to expose my Position and Length properties as FastBinaryReader.BaseStream.Position and FastBinaryReader.BaseStream.Length, so that my existing code will stay compatible with the original BinaryReader class.

How would I go about doing this?

Answer 1

Here's the final implementation, if anyone is interested. Passing this as a Stream object to a BinaryReader, instead of the usual FileStream object, yields about a 45% improvement in speed on my machine, when reading 1000 byte chunks.

Note that the Length param is only accurate when reading, since Length is read in at the start and doesn't change. If you are writing, it will not update as the length of the underlying stream changes.

public class FastFileStream : FileStream
{
    private long _position;
    private long _length;

    public FastFileStream(string path, FileMode fileMode) : base(path, fileMode)
    {
        _position = base.Position;
        _length = base.Length;
    }

    public override long Length
    {
        get { return _length; }
    }

    public override long Position
    {
        get { return _position; }
        set
        {
            base.Position = value;
            _position = value;
        }
    }

    public override long Seek(long offset, SeekOrigin seekOrigin)
    {
        switch (seekOrigin)
        {
            case SeekOrigin.Begin:
                _position = offset;
                break;
            case SeekOrigin.Current:
                _position += offset;
                break;
            case SeekOrigin.End:
                _position = Length + offset;
                break;
        }
        return base.Seek(offset, seekOrigin);
    }

    public override int Read(byte[] array, int offset, int count)
    {
        _position += count;
        return base.Read(array, offset, count);
    }

    public override int ReadByte()
    {
        _position += 1;
        return base.ReadByte();
    }
}

Answer 2

I wouldn't do this exactly the way you have it here.

Consider that you need to expose a property of type Stream (what BinaryReader.BaseStream is). So you 'll need to create your own class deriving from Stream. This class would need to:

• take a reference to FastBinaryReader so that it can override Stream.Length and Stream.Offset by delegating to a FastBinaryReader member
• take a reference to a Stream (the same one passed in the FastBinaryReader constructor) in order to delegate all other operations to that stream (you could have these throw new NotImplementedException() instead, but you never know which library method is going to call them!)

You can imagine how it'd look:

private class StreamWrapper : Stream
{
    private readonly FastBinaryReader reader;

    private readonly Stream baseStream;

    public StreamWrapper(FastBinaryReader reader, Stream baseStream)
    {
        this.reader = reader;
        this.baseStream = baseStream;
    }
    public override long Length
    {
        get { return reader.Length; }
    }

    public override long Position
    {
        get { return reader.Position; }
        set { reader.Position = value; }
    }

    // Override all other Stream virtuals as well
}

This would work, but it seems to me to be slightly clumsy. The logical continuation would be to put the caching in StreamWrapper instead of inside FastBinaryReader itself:

private class StreamWrapper : Stream
{
    private readonly Stream baseStream;

    public StreamWrapper(Stream baseStream)
    {
        this.baseStream = baseStream;
    }

    public override long Length
    {
        get { /* caching implementation */ }
    }

    public override long Position
    {
        get { /* caching implementation */ }
        set { /* caching implementation */ }
    }

    // Override all other Stream virtuals as well
}

This would allow you to use StreamWrapper transparently and keep the caching behavior. But it raises the question: is the Stream you work with so dumb that it doesn't cache this by itself?

And if it isn't, maybe the performance gain you see is the result of that if statement inside ReadBytes and not of caching Length and Position?