Skip to content

Use the Connection Buffer for Writing #134

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

Merged
merged 9 commits into from
Oct 24, 2013
Merged

Use the Connection Buffer for Writing #134

merged 9 commits into from
Oct 24, 2013

Conversation

julienschmidt
Copy link
Member

Instead of allocating multiple small new slices and copy them in one big slice before sending (current approach) or the 2-Pass approach (Pass 1: getting the total size; Pass 2: packing in a new allocated slice with perfect size) in #71, this PR introduces yet another approach:

The connection buffer (currently only used for reading) is used for packing until the the buffer gets too small. At this point we (the builtin func append) start allocating bigger slices. This happens only for very large Queries since the buffer is by default 4KB large and can grow.
When the packing is finished, we can get the total size and write the packet header.

As a result, most packets can be built without making a single allocation, which results in a lot less garbage and a increased execution speed.

// shortcut which can be used if the requested buffer is guaranteed to be
// smaller than defaultBufSize
// Only one buffer (total) can be used at a time.
func (b *buffer) takeSmallBuffer(length int) []byte {
Copy link
Member

Choose a reason for hiding this comment

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

Everywhere this is used in packets.go, it's immediately followed by if data == nil based error handling, so even if it's inlined by the compiler, it always branches twice. b.length != 0 is the only way this can return nil, so I say manually inline all occurrences (and add a comment), use the buffer directly and drop the superflouus if for all 4 occurences.
That's bad from a code style perspective, but the comment should suffice. It will probably be a little faster and not more complex than it is now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Branches are not generally evil. In this case 1 branch miss is to be expected. I think keeping the buffer logic separate is worth it.

@@ -621,13 +643,16 @@ func (rows *mysqlRows) readRow(dest []driver.Value) (err error) {
pos += n
if err == nil {
if !isNull {
if !rows.mc.parseTime {
if !mc.parseTime {
continue
} else {
Copy link
Member

Choose a reason for hiding this comment

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

drop else, deindent (though it's not originally part of this PR)

@arnehormann
Copy link
Member

Done so far (oof), eager to TAL 😀

@arnehormann
Copy link
Member

Independent of else perception differences: LGTM

julienschmidt added a commit that referenced this pull request Oct 24, 2013
Use the Connection Buffer for Writing
@julienschmidt julienschmidt merged commit c87f84b into master Oct 24, 2013
@julienschmidt julienschmidt deleted the write-buffer branch October 24, 2013 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants