refactor sync.Pool usages, improve file reading perf, trim leading '/' in request

Signed-off-by: kim (grufwub) <grufwub@gmail.com>

core/buffer.go

@@ -1,112 +0,0 @@
-package core
-
-import (
-	"bufio"
-	"io"
-	"sync"
-)
-
-var (
-	connBufferedReaderPool sync.Pool
-	connBufferedWriterPool sync.Pool
-	fileBufferedReaderPool sync.Pool
-
-	fileReadBufferPool sync.Pool
-)
-
-func newConnBufferedReaderPool(size int) sync.Pool {
-	return sync.Pool{
-		New: func() interface{} {
-			return bufio.NewReaderSize(nil, size)
-		},
-	}
-}
-
-func newConnBufferedWriterPool(size int) sync.Pool {
-	return sync.Pool{
-		New: func() interface{} {
-			return bufio.NewWriterSize(nil, size)
-		},
-	}
-}
-
-func newFileBufferedReaderPool(size int) sync.Pool {
-	return sync.Pool{
-		New: func() interface{} {
-			return bufio.NewReaderSize(nil, size)
-		},
-	}
-}
-
-func newFileReadBufferPool(size int) sync.Pool {
-	return sync.Pool{
-		New: func() interface{} {
-			return make([]byte, size)
-		},
-	}
-}
-
-func getConnBufferedReader(r io.Reader) *bufio.Reader {
-	// Get buffered reader
-	br := connBufferedReaderPool.Get().(*bufio.Reader)
-
-	// Reset to new reader
-	br.Reset(r)
-
-	// Return!
-	return br
-}
-
-func putConnBufferedReader(br *bufio.Reader) {
-	// Reset to ensure not hanging onto old reader
-	br.Reset(nil)
-
-	// Put back in pool
-	connBufferedReaderPool.Put(br)
-}
-
-func getConnBufferedWriter(w io.Writer) *bufio.Writer {
-	// Get buffered writer
-	bw := connBufferedWriterPool.Get().(*bufio.Writer)
-
-	// Reset to new writer
-	bw.Reset(w)
-
-	// Return!
-	return bw
-}
-
-func putConnBufferedWriter(bw *bufio.Writer) {
-	// Reset to ensure not hanging onto old writer
-	bw.Reset(nil)
-
-	// Put back in pool
-	connBufferedWriterPool.Put(bw)
-}
-
-func getFileBufferedReader(r io.Reader) *bufio.Reader {
-	// Get buffered reader
-	br := fileBufferedReaderPool.Get().(*bufio.Reader)
-
-	// Reset to new reader
-	br.Reset(r)
-
-	// Return!
-	return br
-}
-
-func putFileBufferedReader(br *bufio.Reader) {
-	// Reset to ensure not hanging onto old reader
-	br.Reset(nil)
-
-	// Put back in pool
-	fileBufferedReaderPool.Put(br)
-}
-
-func getFileReadBuffer() []byte {
-	return fileReadBufferPool.Get().([]byte)
-}
-
-func putFileReadBuffer(b []byte) {
-	fileReadBufferPool.Put(b)
-}

core/conn.go

@@ -51,8 +51,8 @@ type conn struct {
 func wrapConn(c net.Conn) *conn {
 	deadlineConn := &deadlineConn{c}
 	return &conn{
-		br: getConnBufferedReader(deadlineConn),
+		br: connBufferedReaderPool.Get(deadlineConn),
-		bw: getConnBufferedWriter(deadlineConn),
+		bw: connBufferedWriterPool.Get(deadlineConn),
 		cl: deadlineConn,
 	}
 }
@@ -60,9 +60,11 @@ func wrapConn(c net.Conn) *conn {
 // ReadLine reads a single line and returns the result, or nil and error
 func (c *conn) ReadLine() ([]byte, Error) {
 	// return slice
-	b := make([]byte, 0)
+	var b []byte
 
-	// Read!
+	// Read! Use this method so we can
+	// ensure we don't perform some insanely
+	// long read
 	for len(b) < connReadMax {
 		// read the line
 		line, isPrefix, err := c.br.ReadLine()
@@ -117,8 +119,8 @@ func (c *conn) Close() Error {
 	err2 := c.cl.Close()
 
 	// Put buffers back
-	putConnBufferedReader(c.br)
+	connBufferedReaderPool.Put(c.br)
-	putConnBufferedWriter(c.bw)
+	connBufferedWriterPool.Put(c.bw)
 
 	// If either errors, wrap. Else return none
 	if err2 != nil {

core/filesystem.go

@@ -102,79 +102,64 @@ func (fs *FileSystemObject) ReadFile(fd *os.File) ([]byte, Error) {
 	// Return slice
 	ret := make([]byte, 0)
 
-	// Read buffers
+	// Get read buffers, defer putting back
-	buf := getFileReadBuffer()
+	br := fileBufferedReaderPool.Get(fd)
-	rd := getFileBufferedReader(fd)
+	defer fileBufferedReaderPool.Put(br)
 
 	// Read through file until null bytes / error
 	for {
-		count, err := rd.Read(buf)
+		// Read line
+		line, err := br.ReadBytes('\n')
 		if err != nil {
 			if err == io.EOF {
+				// EOF, add current to return slice and
+				// break-out. WIll not have hit delim
+				ret = append(ret, line...)
 				break
+			} else {
+				// Bad error, return
+				return nil, WrapError(FileReadErr, err)
 			}
-			return nil, WrapError(FileReadErr, err)
 		}
 
-		ret = append(ret, buf[:count]...)
+		// Add current line to return slice, skip
-
+		// final byte which is '\n'
-		if count < fileReadBufSize {
+		ret = append(ret, line[:len(line)-1]...)
-			break
-		}
 	}
 
-	// Put back buffers
-	putFileReadBuffer(buf)
-	putFileBufferedReader(rd)
-
 	// Return!
 	return ret, nil
 }
 
 // ScanFile scans a supplied file at file descriptor, using iterator function
 func (fs *FileSystemObject) ScanFile(fd *os.File, iterator func(string) bool) Error {
-	// Read buffers
+	// Get read buffer, defer putting back
-	rd := getFileBufferedReader(fd)
+	br := fileBufferedReaderPool.Get(fd)
+	defer fileBufferedReaderPool.Put(br)
 
 	// Iterate through file!
 	for {
-		// Line buffer
+		// Read a line
-		b := make([]byte, 0)
+		line, err := br.ReadString('\n')
-
+		if err != nil {
-		// Read until line-end, or file end!
+			if err == io.EOF {
-		done := false
+				// Reached end of file, perform final iteration
-		for {
+				// and break-out. Will not have hit delim
-			// Read a line
+				iterator(line)
-			line, isPrefix, err := rd.ReadLine()
-			if err != nil {
-				if err == io.EOF {
-					done = true
-					break
-				}
-				return WrapError(FileReadErr, err)
-			}
-
-			// Append to line buffer
-			b = append(b, line...)
-
-			// If not isPrefix, we can break-out
-			if !isPrefix {
 				break
+			} else {
+				// Bad error, return
+				return WrapError(FileReadErr, err)
 			}
 		}
 
-		// Run scan iterator on this line, break-out if requested
+		// Run scan iterator on this line, breaking out if requested,
-		if !iterator(string(b)) || done {
+		// skipping final byte which is '\n'
+		if !iterator(line[:len(line)-1]) {
 			break
 		}
-
-		// Empty the slice!
-		b = nil
 	}
 
-	// Put back buffers
-	putFileBufferedReader(rd)
-
 	// Return no errors :)
 	return nil
 }

core/host.go

@@ -4,8 +4,8 @@ var (
 	// Root stores the server's root directory
 	Root string
 
-	// BindAddr stores the server's bound IP
+	// Bind stores the server's bound IP
-	BindAddr string
+	Bind string
 
 	// Hostname stores the host's outward hostname
 	Hostname string

core/pool.go

@@ -0,0 +1,108 @@
+package core
+
+import (
+	"bufio"
+	"io"
+	"sync"
+)
+
+var (
+	connBufferedReaderPool *bufferedReaderPool
+	connBufferedWriterPool *bufferedWriterPool
+	fileBufferedReaderPool *bufferedReaderPool
+	fileBufferPool         *bufferPool
+)
+
+type bufferPool struct {
+	pool sync.Pool
+}
+
+func newBufferPool(size int) *bufferPool {
+	return &bufferPool{
+		pool: sync.Pool{
+			New: func() interface{} {
+				return make([]byte, size)
+			},
+		},
+	}
+}
+
+func (bp *bufferPool) Get() []byte {
+	// Just return and cast a buffer
+	return bp.pool.Get().([]byte)
+}
+
+func (bp *bufferPool) Put(b []byte) {
+	// Just put back in pool
+	bp.pool.Put(b)
+}
+
+type bufferedReaderPool struct {
+	pool sync.Pool
+}
+
+func newBufferedReaderPool(size int) *bufferedReaderPool {
+	return &bufferedReaderPool{
+		pool: sync.Pool{
+			New: func() interface{} {
+				return bufio.NewReaderSize(nil, size)
+			},
+		},
+	}
+}
+
+func (bp *bufferedReaderPool) Get(r io.Reader) *bufio.Reader {
+	// Get a buffered reader from the pool
+	br := bp.pool.Get().(*bufio.Reader)
+
+	// Reset to use our new reader!
+	br.Reset(r)
+
+	// Return
+	return br
+}
+
+func (bp *bufferedReaderPool) Put(br *bufio.Reader) {
+	// We must reset again here to ensure
+	// we don't mess with GC with unused underlying
+	// reader.
+	br.Reset(nil)
+
+	// Put back in the pool
+	bp.pool.Put(br)
+}
+
+type bufferedWriterPool struct {
+	pool sync.Pool
+}
+
+func newBufferedWriterPool(size int) *bufferedWriterPool {
+	return &bufferedWriterPool{
+		pool: sync.Pool{
+			New: func() interface{} {
+				return bufio.NewWriterSize(nil, size)
+			},
+		},
+	}
+}
+
+func (bp *bufferedWriterPool) Get(w io.Writer) *bufio.Writer {
+	// Get a buffered writer from the pool
+	bw := bp.pool.Get().(*bufio.Writer)
+
+	// Reset to user our new writer
+	bw.Reset(w)
+
+	// Return
+	return bw
+}
+
+func (bp *bufferedWriterPool) Put(bw *bufio.Writer) {
+	// We must reset again here to ensure
+	// we don't mess with GC with unused underlying
+	// writer.
+	bw.Reset(nil)
+
+	// Put back in the pool
+	bp.pool.Put(bw)
+}

core/server.go

@@ -30,7 +30,7 @@ func ParseFlagsAndSetup(proto string, errorMessageFunc func(ErrorCode) string) {
 	sysLog := flag.String(sysLogFlagStr, "stdout", sysLogDescStr)
 	accLog := flag.String(accLogFlagStr, "stdout", accLogDescStr)
 	flag.StringVar(&Root, rootFlagStr, "/var/gopher", rootDescStr)
-	flag.StringVar(&BindAddr, bindAddrFlagStr, "", bindAddrDescStr)
+	flag.StringVar(&Bind, bindFlagStr, "", bindDescStr)
 	flag.StringVar(&Hostname, hostnameFlagStr, "localhost", hostnameDescStr)
 	port := flag.Uint(portFlagStr, 70, portDescStr)
 	fwdPort := flag.Uint(fwdPortFlagStr, 0, fwdPortDescStr)
@@ -73,10 +73,10 @@ func ParseFlagsAndSetup(proto string, errorMessageFunc func(ErrorCode) string) {
 
 	// Check valid values for BindAddr and Hostname
 	if Hostname == "" {
-		if BindAddr == "" {
+		if Bind == "" {
-			SystemLog.Fatal(hostnameBindAddrEmptyStr)
+			SystemLog.Fatal(hostnameBindEmptyStr)
 		}
-		Hostname = BindAddr
+		Hostname = Bind
 	}
 
 	// Change to server directory
@@ -97,16 +97,16 @@ func ParseFlagsAndSetup(proto string, errorMessageFunc func(ErrorCode) string) {
 
 	// Setup listener
 	var err Error
-	serverListener, err = newListener(BindAddr, Port)
+	serverListener, err = newListener(Bind, Port)
 	if err != nil {
-		SystemLog.Fatal(listenerBeginFailStr, protocol, Hostname, FwdPort, BindAddr, Port, err.Error())
+		SystemLog.Fatal(listenerBeginFailStr, protocol, Hostname, FwdPort, Bind, Port, err.Error())
 	}
 
 	// Setup the sync pools
-	connBufferedReaderPool = newConnBufferedReaderPool(int(*cReadBuf))
+	connBufferedReaderPool = newBufferedReaderPool(int(*cReadBuf))
-	connBufferedWriterPool = newConnBufferedWriterPool(int(*cWriteBuf))
+	connBufferedWriterPool = newBufferedWriterPool(int(*cWriteBuf))
-	fileBufferedReaderPool = newFileBufferedReaderPool(int(*fReadBuf))
+	fileBufferedReaderPool = newBufferedReaderPool(int(*fReadBuf))
-	fileReadBufferPool = newFileReadBufferPool(int(*fReadBuf))
+	fileBufferPool = newBufferPool(int(*fReadBuf))
 
 	// Conn read max
 	connReadMax = int(*cReadMax)
@@ -197,7 +197,7 @@ func Start(serve func(*Client)) {
 	go FileSystem.StartMonitor()
 
 	// Start the listener
-	SystemLog.Info(listeningOnStr, protocol, Hostname, FwdPort, BindAddr, Port)
+	SystemLog.Info(listeningOnStr, protocol, Hostname, FwdPort, Bind, Port)
 	go func() {
 		for {
 			client, err := serverListener.Accept()

core/string_constants.go

@@ -11,8 +11,8 @@ const (
 	rootFlagStr = "root"
 	rootDescStr = "Server root directory"
 
-	bindAddrFlagStr = "bind"
+	bindFlagStr = "bind"
-	bindAddrDescStr = "IP address to bind to"
+	bindDescStr = "IP address to bind to"
 
 	hostnameFlagStr = "hostname"
 	hostnameDescStr = "Server hostname (FQDN)"
@@ -83,7 +83,7 @@ const (
 
 // Log string constants
 const (
-	hostnameBindAddrEmptyStr = "At least one of hostname or bind-addr must be non-empty!"
+	hostnameBindEmptyStr = "At least one of hostname or bind-addr must be non-empty!"
 
 	chDirStr    = "Entered server dir: %s"
 	chDirErrStr = "Error entering server directory: %s"

gopher/server.go

@@ -16,8 +16,8 @@ func serve(client *core.Client) {
 		return
 	}
 
-	// Convert to string
+	// Convert to string + remove leading '/'
-	line := string(received)
+	line := strings.TrimPrefix(string(received), "/")
 
 	// If prefixed by 'URL:' send a redirect
 	lenBefore := len(line)