-
Notifications
You must be signed in to change notification settings - Fork 2.3k
zero-copy buffer #55
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
zero-copy buffer #55
Conversation
No extra allocation when refilling the buffer Closes #52
if err != nil { | ||
errLog.Print(err.Error()) | ||
return nil, driver.ErrBadConn | ||
} | ||
|
||
// Packet Length [24 bit] | ||
pktLen := uint32(data[0]) | uint32(data[1])<<8 | uint32(data[2])<<16 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leave as is, cast to int in L54 readNext?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would be the benefit of that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
honestly - none. Just one change less (and I dislike parens around long expressions)
LGTM |
Continued from #52
This approach removes the 2nd buffer (date / packet buffer). Instead we work with slices directly from the buffer. This results in more or less zero copy.
Performance:
Before
After
I worked on such a "zero copy buffer" before but stopped when I found a reason why this concept couldn't work - at least I assumed it the be a reason.
Until now I found no case where the new buffer causes problems, but please test it as much as you can.
Especially cases where
sql.RawBytes
is used excessively could be interesting.