Skip to content

Replaced use of custom buffer type with stdlib implementation #51

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

Closed
wants to merge 2 commits into from

Conversation

balasanjay
Copy link

No description provided.

@balasanjay
Copy link
Author

@julienschmidt, please take a look at this change when you get the chance.

I have an incoming pull request that lowers the number of allocations that is much nicer when the buffer used implements io.Reader.

@julienschmidt
Copy link
Member

The driver originally used the bytes.Buffer but was replaced with the custom buffer for performance improvements in ccad956.
But I'm open to undo this if the performance losses are negligible when reusing the buffer. I'm testing and benchmarking now.

@julienschmidt
Copy link
Member

Unfortunately, I notice performance degradation of ~ 8-9% (avg.) in Go 1.1beta2 and ~10% in Go 1.0.3.
You can play around with SQL-Benchmark if you want to try to optimize this.
I like the idea of this change, but from my point of view the current performance degradation is too large.

@julienschmidt
Copy link
Member

Sorry i meant the #52 change.
I'm not going to merge this change here on its own so I'm closing this. Please continue the discussion at #52

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.

2 participants