New custom stream classes replacing BufferedStream and MemoryStream

Hi all,

Per discussion with @roji, I implemented some Stream classes to replace BufferedStream and MemoryStream, and got them working in Npgsql.

This is really preliminary work, and just the start of a much bigger project that I don't have time to finish, but i think it provides part of the basis needed for Npgsql to really take control of its buffer management.

i did not implement a buffer pool, which may prove to be another important aspect of this project, but a buffer pool is really quite trivial to write, and adapting NpgsqlMemoryStream to it would also be trivial.

The hard, tedious work ahead will lie in refactoring Npgsql to extend and use these new streams to eliminate a lot more allocations and copy operations, partly by taking advantage of the fact that we can have full access to their buffers.

-Glen

Yep, it's busted .I'm fixing this up now.

-Glen

On Fri, Oct 31, 2014 at 12:29 AM, Shay Rojansky <notifications@github.com>
wrote:

> In Npgsql/NpgsqlBufferedStream.cs:
>
> > +        protected override bool PopulateReadBuffer(int count)
> > +        {
> > +            if (readBufferPosition + count <= readBufferCapacity)
> > +            {
> > +                return true;
> > +            }
> > +
> > +            if (readBuffer == null)
> > +            {
> > +                readBuffer = new byte[BufferSize];
> > +            }
> > +
> > +            if (readBufferPosition < readBufferCapacity)
> > +            {
> > +                Array.Copy(readBuffer, readBufferPosition, readBuffer, 0, readBufferCapacity - readBufferPosition);
> > +                readBufferCapacity = readBufferCapacity - readBufferPosition;
>
> I also think there's a problem with PopulateReadBuffer.
>
> Right now it is being called exactly once (e.g. by ReadInt64()), and
> itself performs a single stream read. There is no guarantee that the int64
> will be read in a single underlying Stream.Read() - this depends on things
> like how data is split into packets. The way the code currently is,
> PopulateReadBuffer will only read half the int64 and return false, and
> ReadInt64() will throw an EndOfStreamException, which is incorrect behavior.
>
> Like @Emill <https://github.com/Emill> says, PopulateReadBuffer should
> read continuously until at least the requested number of bytes is available
> in the buffer...
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/npgsql/npgsql/pull/388/files#r19654722>.
>