From 61026b7c21bc88079489858a2d9c7c7f708a0348 Mon Sep 17 00:00:00 2001 From: Jack Christensen Date: Sat, 29 Apr 2017 11:55:14 -0500 Subject: [PATCH] Reduce allocations and copies in pgproto3 Altered chunkreader to never reuse memory. Altered pgproto3 to to copy memory when decoding. Renamed UnmarshalBinary to Decode because of changed semantics. --- chunkreader.go | 25 ++++--------------------- chunkreader_test.go | 44 ++++++-------------------------------------- 2 files changed, 10 insertions(+), 59 deletions(-) diff --git a/chunkreader.go b/chunkreader.go index f9d6555c..f8d437b2 100644 --- a/chunkreader.go +++ b/chunkreader.go @@ -9,14 +9,12 @@ type ChunkReader struct { buf []byte rp, wp int // buf read position and write position - taken bool options Options } type Options struct { MinBufLen int // Minimum buffer length - BlockLen int // Increments to expand buffer (e.g. a 8000 byte request with a BlockLen of 1024 would yield a buffer len of 8192) } func NewChunkReader(r io.Reader) *ChunkReader { @@ -32,9 +30,6 @@ func NewChunkReaderEx(r io.Reader, options Options) (*ChunkReader, error) { if options.MinBufLen == 0 { options.MinBufLen = 4096 } - if options.BlockLen == 0 { - options.BlockLen = 512 - } return &ChunkReader{ r: r, @@ -43,8 +38,8 @@ func NewChunkReaderEx(r io.Reader, options Options) (*ChunkReader, error) { }, nil } -// Next returns buf filled with the next n bytes. buf is only valid until the -// next call to Next. If an error occurs, buf will be nil. +// Next returns buf filled with the next n bytes. If an error occurs, buf will +// be nil. func (r *ChunkReader) Next(n int) (buf []byte, err error) { // n bytes already in buf if (r.wp - r.rp) >= n { @@ -56,17 +51,12 @@ func (r *ChunkReader) Next(n int) (buf []byte, err error) { // available space in buf is less than n if len(r.buf) < n { r.copyBufContents(r.newBuf(n)) - r.taken = false } // buf is large enough, but need to shift filled area to start to make enough contiguous space minReadCount := n - (r.wp - r.rp) if (len(r.buf) - r.wp) < minReadCount { - newBuf := r.buf - if r.taken { - newBuf = r.newBuf(n) - r.taken = false - } + newBuf := r.newBuf(n) r.copyBufContents(newBuf) } @@ -79,20 +69,13 @@ func (r *ChunkReader) Next(n int) (buf []byte, err error) { return buf, nil } -// KeepLast prevents the last data retrieved by Next from being reused by the -// ChunkReader. -func (r *ChunkReader) KeepLast() { - r.taken = true -} - func (r *ChunkReader) appendAtLeast(fillLen int) error { n, err := io.ReadAtLeast(r.r, r.buf[r.wp:], fillLen) r.wp += n return err } -func (r *ChunkReader) newBuf(min int) []byte { - size := ((min / r.options.BlockLen) + 1) * r.options.BlockLen +func (r *ChunkReader) newBuf(size int) []byte { if size < r.options.MinBufLen { size = r.options.MinBufLen } diff --git a/chunkreader_test.go b/chunkreader_test.go index 9c19ff4a..3be07e3c 100644 --- a/chunkreader_test.go +++ b/chunkreader_test.go @@ -7,7 +7,7 @@ import ( func TestChunkReaderNextDoesNotReadIfAlreadyBuffered(t *testing.T) { server := &bytes.Buffer{} - r, err := NewChunkReaderEx(server, Options{MinBufLen: 4, BlockLen: 2}) + r, err := NewChunkReaderEx(server, Options{MinBufLen: 4}) if err != nil { t.Fatal(err) } @@ -44,7 +44,7 @@ func TestChunkReaderNextDoesNotReadIfAlreadyBuffered(t *testing.T) { func TestChunkReaderNextExpandsBufAsNeeded(t *testing.T) { server := &bytes.Buffer{} - r, err := NewChunkReaderEx(server, Options{MinBufLen: 4, BlockLen: 2}) + r, err := NewChunkReaderEx(server, Options{MinBufLen: 4}) if err != nil { t.Fatal(err) } @@ -59,14 +59,14 @@ func TestChunkReaderNextExpandsBufAsNeeded(t *testing.T) { if bytes.Compare(n1, src[0:5]) != 0 { t.Fatalf("Expected read bytes to be %v, but they were %v", src[0:5], n1) } - if len(r.buf) != 6 { - t.Fatalf("Expected len(r.buf) to be %v, but it was %v", 6, len(r.buf)) + if len(r.buf) != 5 { + t.Fatalf("Expected len(r.buf) to be %v, but it was %v", 5, len(r.buf)) } } -func TestChunkReaderNextReusesBuf(t *testing.T) { +func TestChunkReaderDoesNotReuseBuf(t *testing.T) { server := &bytes.Buffer{} - r, err := NewChunkReaderEx(server, Options{MinBufLen: 4, BlockLen: 1}) + r, err := NewChunkReaderEx(server, Options{MinBufLen: 4}) if err != nil { t.Fatal(err) } @@ -90,38 +90,6 @@ func TestChunkReaderNextReusesBuf(t *testing.T) { t.Fatalf("Expected read bytes to be %v, but they were %v", src[4:8], n2) } - if bytes.Compare(n1, src[4:8]) != 0 { - t.Fatalf("Expected Next to have reused buf, %v found instead of %v", src[4:8], n1) - } -} - -func TestChunkReaderKeepLastPreventsBufReuse(t *testing.T) { - server := &bytes.Buffer{} - r, err := NewChunkReaderEx(server, Options{MinBufLen: 4, BlockLen: 1}) - if err != nil { - t.Fatal(err) - } - - src := []byte{1, 2, 3, 4, 5, 6, 7, 8} - server.Write(src) - - n1, err := r.Next(4) - if err != nil { - t.Fatal(err) - } - if bytes.Compare(n1, src[0:4]) != 0 { - t.Fatalf("Expected read bytes to be %v, but they were %v", src[0:4], n1) - } - r.KeepLast() - - n2, err := r.Next(4) - if err != nil { - t.Fatal(err) - } - if bytes.Compare(n2, src[4:8]) != 0 { - t.Fatalf("Expected read bytes to be %v, but they were %v", src[4:8], n2) - } - if bytes.Compare(n1, src[0:4]) != 0 { t.Fatalf("Expected KeepLast to prevent Next from overwriting buf, expected %v but it was %v", src[0:4], n1) }