Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't put huge buffers back into the pool #18

Merged
merged 1 commit into from
Oct 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 10 additions & 13 deletions golog.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@
//
// - The following linker flag is set
//
// -X github.com/getlantern/golog.linkerFlagEnableTraceThroughLinker=true
// -X github.com/getlantern/golog.linkerFlagEnableTraceThroughLinker=true
//
// - Optionally, you can also set a comma-separated list of prefixes to trace
// through the following linker flag
// - Optionally, you can also set a comma-separated list of prefixes to trace
// through the following linker flag
//
// -X github.com/getlantern/golog.linkerFlagTracePrefixes=prefix1,prefix2
// -X github.com/getlantern/golog.linkerFlagTracePrefixes=prefix1,prefix2
//
// - Or, alternatively, trace logging can also be enable if "TRACE=true"
// environment variable is set
// - Or, alternatively, trace logging can also be enable if "TRACE=true"
// environment variable is set
//
// - Optionally, you can also set a comma-separated list of prefixes to trace
// through the "TRACE" environment variable like this: "TRACE=prefix1,prefix2"
// - Optionally, you can also set a comma-separated list of prefixes to trace
// through the "TRACE" environment variable like this: "TRACE=prefix1,prefix2"
//
// A stack dump will be printed after the message if "PRINT_STACK=true".
package golog
Expand All @@ -39,7 +39,6 @@ import (
"github.com/getlantern/errors"
"github.com/getlantern/hidden"
"github.com/getlantern/ops"
"github.com/oxtoacart/bpool"
)

const (
Expand Down Expand Up @@ -68,8 +67,6 @@ var (
reporters []ErrorReporter
reportersMutex sync.RWMutex

bufferPool = bpool.NewBufferPool(200)

onFatal atomic.Value

// enableTraceThroughLinker is set through a linker flag. It's used to
Expand Down Expand Up @@ -470,8 +467,8 @@ func argToString(arg interface{}) string {
if ml, isMultiline := arg.(MultiLine); !isMultiline {
return fmt.Sprintf("%v", arg)
} else {
buf := bufferPool.Get()
defer bufferPool.Put(buf)
buf := getBuffer()
defer returnBuffer(buf)
mlp := ml.MultiLinePrinter()
for {
more := mlp(buf)
Expand Down
4 changes: 2 additions & 2 deletions json_output.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ func (o *jsonOutput) print(writer io.Writer, prefix string, skipFrames int, prin
cleanPrefix := prefix[0 : len(prefix)-2] // prefix contains ': ' at the end, strip it
event := Event{Component: cleanPrefix, Severity: severity, Caller: o.caller(skipFrames), Context: values}
if printStack {
buf := bufferPool.Get()
defer bufferPool.Put(buf)
buf := getBuffer()
defer returnBuffer(buf)
_ = writeStack(buf, o.pc)
event.Stack = buf.String()
}
Expand Down
29 changes: 29 additions & 0 deletions pool.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package golog

import (
"bytes"

"github.com/oxtoacart/bpool"
)

const (
// Together, these bound the buffer pool to 307 KB
bufferPoolSize = 200
maxBufferSize = 1024
)

var (
_bufferPool = bpool.NewBufferPool(bufferPoolSize)
)

func getBuffer() *bytes.Buffer {
return _bufferPool.Get()
}

func returnBuffer(buf *bytes.Buffer) bool {
if buf.Cap() <= maxBufferSize {
_bufferPool.Put(buf)
return true
}
return false
}
21 changes: 21 additions & 0 deletions pool_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package golog

import (
"testing"

"github.com/stretchr/testify/require"
)

func TestPool(t *testing.T) {
buf := _bufferPool.Get()
require.NotNil(t, buf)
// Write 768 bytes (the max before the buffer's capacity exceeds )
for i := 0; i < maxBufferSize; i++ {
buf.WriteByte('C')
}
require.True(t, returnBuffer(buf))
for i := 0; i <= maxBufferSize; i++ {
buf.WriteByte('C')
}
require.False(t, returnBuffer(buf))
}
4 changes: 2 additions & 2 deletions text_output.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ func (o *textOutput) Debug(prefix string, skipFrames int, printStack bool, sever
}

func (o *textOutput) print(writer io.Writer, prefix string, skipFrames int, printStack bool, severity string, arg interface{}, values map[string]interface{}) {
buf := bufferPool.Get()
defer bufferPool.Put(buf)
buf := getBuffer()
defer returnBuffer(buf)

GetPrepender()(buf)
linePrefix := o.linePrefix(prefix, skipFrames)
Expand Down