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

Reduce RingBuffer heap allocation #205

Merged
merged 1 commit into from Mar 2, 2020

Conversation

chrisxue815
Copy link
Contributor

@chrisxue815 chrisxue815 commented Aug 29, 2019

Hi,

Thanks for creating this library. Great work!

It seems to me that in RingBuffer we can reduce the number of heap allocations by changing nodes
from an array of node pointers: type nodes []*node
to an array of node values: type nodes []node

It should also increase locality, cache efficiency and increase performance in theory.

As you can see the benchmark results below, this change reduced ns/op, B/ops and allocs/op of BenchmarkRBAllocation a lot, while having no significant impact on other benchmarks.

type nodes []*node
$ go test -bench BenchmarkRB.* -benchmem
goos: linux
goarch: amd64
pkg: github.com/chrisxue815/go-datastructures/queue
BenchmarkRBLifeCycle-8                   2000000               889 ns/op             200 B/op          3 allocs/op
BenchmarkRBLifeCycleContention-8          100000             12167 ns/op            1999 B/op         29 allocs/op
BenchmarkRBPut-8                        50000000                33.6 ns/op             8 B/op          0 allocs/op
BenchmarkRBGet-8                        100000000               12.1 ns/op             0 B/op          0 allocs/op
BenchmarkRBAllocation-8                    50000             28084 ns/op           40960 B/op       1025 allocs/op
PASS
ok      github.com/chrisxue815/go-datastructures/queue  20.987s
type nodes []node
$ go test -bench BenchmarkRB.* -benchmem
goos: linux
goarch: amd64
pkg: github.com/chrisxue815/go-datastructures/queue
BenchmarkRBLifeCycle-8                   2000000               875 ns/op             200 B/op          3 allocs/op
BenchmarkRBLifeCycleContention-8          200000             12002 ns/op            1999 B/op         29 allocs/op
BenchmarkRBPut-8                        50000000                26.6 ns/op             8 B/op          0 allocs/op
BenchmarkRBGet-8                        100000000               13.6 ns/op             0 B/op          0 allocs/op
BenchmarkRBAllocation-8                   500000              3833 ns/op           24576 B/op          1 allocs/op
PASS
ok      github.com/chrisxue815/go-datastructures/queue  16.446s

@aviary-wf
Copy link

Security Insights

No security relevant content was detected by automated scans.

Action Items

  • Review PR for security impact; comment "security review required" if needed or unsure
  • Verify aviary.yaml coverage of security relevant code

Questions or Comments? Reach out on Slack: #support-infosec.

@chrisxue815 chrisxue815 marked this pull request as ready for review August 29, 2019 16:25
@brianshannan-wf
Copy link
Collaborator

@dustinhiatt-wf

@dustinhiatt-wf
Copy link
Contributor

@Workiva/release-management-p

@dustinhiatt-wf dustinhiatt-wf merged commit 97fa74f into Workiva:master Mar 2, 2020
@teresarevious-wf
Copy link

RM +1 dependencies verified when consumed in tier 1 repo

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

Successfully merging this pull request may close these issues.

None yet

7 participants