From 529d28e62e4065a3d84dc8e55f9013dd822459cc Mon Sep 17 00:00:00 2001 From: "kim (grufwub)" Date: Fri, 23 Oct 2020 20:59:09 +0100 Subject: [PATCH] refactor sync.Pool usages, improve file reading perf, trim leading '/' in request Signed-off-by: kim (grufwub) --- core/buffer.go | 112 --------------------------------------- core/conn.go | 14 ++--- core/filesystem.go | 75 +++++++++++--------------- core/host.go | 4 +- core/pool.go | 108 +++++++++++++++++++++++++++++++++++++ core/server.go | 22 ++++---- core/string_constants.go | 6 +-- gopher/server.go | 4 +- 8 files changed, 164 insertions(+), 181 deletions(-) delete mode 100644 core/buffer.go create mode 100644 core/pool.go diff --git a/core/buffer.go b/core/buffer.go deleted file mode 100644 index 9142fde..0000000 --- a/core/buffer.go +++ /dev/null @@ -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) -} diff --git a/core/conn.go b/core/conn.go index 312fc4b..625251b 100644 --- a/core/conn.go +++ 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), - bw: getConnBufferedWriter(deadlineConn), + br: connBufferedReaderPool.Get(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) - putConnBufferedWriter(c.bw) + connBufferedReaderPool.Put(c.br) + connBufferedWriterPool.Put(c.bw) // If either errors, wrap. Else return none if err2 != nil { diff --git a/core/filesystem.go b/core/filesystem.go index e073cbf..62158f5 100644 --- a/core/filesystem.go +++ b/core/filesystem.go @@ -102,79 +102,64 @@ func (fs *FileSystemObject) ReadFile(fd *os.File) ([]byte, Error) { // Return slice ret := make([]byte, 0) - // Read buffers - buf := getFileReadBuffer() - rd := getFileBufferedReader(fd) + // Get read buffers, defer putting back + br := fileBufferedReaderPool.Get(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]...) - - if count < fileReadBufSize { - break - } + // Add current line to return slice, skip + // final byte which is '\n' + ret = append(ret, line[:len(line)-1]...) } - // 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 - rd := getFileBufferedReader(fd) + // Get read buffer, defer putting back + br := fileBufferedReaderPool.Get(fd) + defer fileBufferedReaderPool.Put(br) // Iterate through file! for { - // Line buffer - b := make([]byte, 0) - - // Read until line-end, or file end! - done := false - for { - // Read a line - line, isPrefix, err := rd.ReadLine() - if err != nil { - if err == io.EOF { - done = true - break - } + // Read a line + line, err := br.ReadString('\n') + if err != nil { + if err == io.EOF { + // Reached end of file, perform final iteration + // and break-out. Will not have hit delim + iterator(line) + break + } else { + // Bad error, return return WrapError(FileReadErr, err) } - - // Append to line buffer - b = append(b, line...) - - // If not isPrefix, we can break-out - if !isPrefix { - break - } } - // Run scan iterator on this line, break-out if requested - if !iterator(string(b)) || done { + // Run scan iterator on this line, breaking out if requested, + // 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 } diff --git a/core/host.go b/core/host.go index 0a34383..be482a6 100644 --- a/core/host.go +++ b/core/host.go @@ -4,8 +4,8 @@ var ( // Root stores the server's root directory Root string - // BindAddr stores the server's bound IP - BindAddr string + // Bind stores the server's bound IP + Bind string // Hostname stores the host's outward hostname Hostname string diff --git a/core/pool.go b/core/pool.go new file mode 100644 index 0000000..5d3b1ed --- /dev/null +++ b/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) +} diff --git a/core/server.go b/core/server.go index 5a9396d..0e265b4 100644 --- a/core/server.go +++ b/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 == "" { - SystemLog.Fatal(hostnameBindAddrEmptyStr) + if Bind == "" { + 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)) - connBufferedWriterPool = newConnBufferedWriterPool(int(*cWriteBuf)) - fileBufferedReaderPool = newFileBufferedReaderPool(int(*fReadBuf)) - fileReadBufferPool = newFileReadBufferPool(int(*fReadBuf)) + connBufferedReaderPool = newBufferedReaderPool(int(*cReadBuf)) + connBufferedWriterPool = newBufferedWriterPool(int(*cWriteBuf)) + fileBufferedReaderPool = newBufferedReaderPool(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() diff --git a/core/string_constants.go b/core/string_constants.go index 4e5f295..8264fd3 100644 --- a/core/string_constants.go +++ b/core/string_constants.go @@ -11,8 +11,8 @@ const ( rootFlagStr = "root" rootDescStr = "Server root directory" - bindAddrFlagStr = "bind" - bindAddrDescStr = "IP address to bind to" + bindFlagStr = "bind" + 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" diff --git a/gopher/server.go b/gopher/server.go index 434a424..f4eaede 100644 --- a/gopher/server.go +++ b/gopher/server.go @@ -16,8 +16,8 @@ func serve(client *core.Client) { return } - // Convert to string - line := string(received) + // Convert to string + remove leading '/' + line := strings.TrimPrefix(string(received), "/") // If prefixed by 'URL:' send a redirect lenBefore := len(line)