Skip to content
This repository has been archived by the owner on Jan 27, 2022. It is now read-only.

Cap internal buffer size for large array performance #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rykov
Copy link

@rykov rykov commented Nov 23, 2017

I was seeing some performance issues with marshalling large arrays where just a few thousands of complex objects were marshalled for 20+ seconds.

Digging deeper, it seems to be due to internal buffer management and step-wise growing of the buffer for each operation that extends the buffer beyond initial 128 bytes.

rmarsh/generator.go

Lines 538 to 544 in 69f255a

if len(gen.buf) < gen.bufn+sz {
newBuf := make([]byte, gen.bufn+sz)
if gen.bufn > 0 {
copy(newBuf, gen.buf)
}
gen.buf = newBuf
}

Doing copy for each operation on smaller buffer sizes has negligible performance impact, however, as the buffer size grows, the impact becomes significant. In my large-array situation, given that all array elements are being buffered until EndArray is called, this problem goes from bad to worse as array size grows.

This PR flushes the buffer at 512 to limit its growth and reduce the performance penalty.

Performance before the changes:

$ go test -bench BenchmarkGenLargeArray *.go
goos: darwin
goarch: amd64
BenchmarkGenLargeArray-4   	       1	2112491558 ns/op
PASS
ok  	command-line-arguments	2.431s

Performance after the changes (almost 1900 times faster):

$ go test -bench BenchmarkGenLargeArray *.go
goos: darwin
goarch: amd64
BenchmarkGenLargeArray-4   	    2000	   1126527 ns/op
PASS
ok  	command-line-arguments	2.614s

@rykov
Copy link
Author

rykov commented Nov 23, 2017

As an extension of this PR, I want to open a discussion on whether using a standard library buffer (bytes.Buffer) or a buffered writer (bufio.Writer) rather than manually managing the buffer would be more performant and easier to maintain in the long run.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant