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 allocations using nil cursors and literal value cursors #8652

Merged
merged 2 commits into from
Aug 1, 2017

Conversation

stuartcarnie
Copy link
Contributor

  • Replaces individual literal cursors with a single interface value. This has the benefit that the value is immediately wrapped in an iface, removing convT2I calls and subsequent allocations every time the value is read.
  • Statically allocates the nil value cursors, saving further allocations as they are stateless and safe for concurrent use.
benchmark                           old ns/op     new ns/op     delta
BenchmarkIntegerIterator_Next-8     82.8          22.7          -72.58%

benchmark                           old allocs     new allocs     delta
BenchmarkIntegerIterator_Next-8     3              0              -100.00%

benchmark                           old bytes     new bytes     delta
BenchmarkIntegerIterator_Next-8     32            0             -100.00%
Required for all non-trivial PRs
  • Rebased/mergable
  • Tests pass

```
benchmark                           old ns/op     new ns/op     delta
BenchmarkIntegerIterator_Next-8     82.8          22.7          -72.58%

benchmark                           old allocs     new allocs     delta
BenchmarkIntegerIterator_Next-8     3              0              -100.00%

benchmark                           old bytes     new bytes     delta
BenchmarkIntegerIterator_Next-8     32            0             -100.00%
```
@jwilder jwilder requested review from jsternberg and removed request for jwilder July 31, 2017 20:13
@jwilder
Copy link
Contributor

jwilder commented Jul 31, 2017

@jsternberg You are more familiar with this code than I am. LGTM though.

Copy link
Contributor

@jsternberg jsternberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the changelog and this looks good.

@stuartcarnie
Copy link
Contributor Author

@jsternberg / @jwilder what should we put in the CHANGELOG for this one?

@jsternberg
Copy link
Contributor

I'm not really sure. Likely goes in the features section and references this PR. Just a one sentence blurb about optimizing the memory usage for some cursors works. Enough for a person who is perusing the changelog to click the link for more information if they desire.

@stuartcarnie stuartcarnie merged commit 5449285 into master Aug 1, 2017
@stuartcarnie stuartcarnie deleted the sgc-literal-cursor branch August 1, 2017 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants