-
Notifications
You must be signed in to change notification settings - Fork 2.3k
optimize readPacket #1705
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
optimize readPacket #1705
Conversation
WalkthroughThis change refactors the buffer and compressed IO reading logic by simplifying method signatures and internal data flow. The Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant mysqlConn
participant buffer
Client->>mysqlConn: readPacket()
mysqlConn->>mysqlConn: readNext(n)
mysqlConn->>buffer: readNext(n)
buffer-->>mysqlConn: []byte
mysqlConn-->>Client: packet data
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
⏰ Context from checks skipped due to timeout of 90000ms (20)
🔇 Additional comments (10)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
data, err := readNext(4, mc.readWithTimeout) | ||
data, err := readNext(4) |
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.
go compiler allocated mc.readWithTimeout
method trampoline object.
@@ -85,7 +96,7 @@ func (mc *mysqlConn) readPacket() ([]byte, error) { | |||
} | |||
|
|||
// read packet body [pktLen bytes] | |||
data, err = readNext(pktLen, mc.readWithTimeout) | |||
data, err = readNext(pktLen) |
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.
ditto.
// read n bytes from mc.buf | ||
func (mc *mysqlConn) readNext(n int) ([]byte, error) { | ||
if mc.buf.len() < n { | ||
err := mc.buf.fill(n, mc.readWithTimeout) |
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.
Go compiler inlines mc.buff.fill. It doesn't allocate mc.readWithTimeout.
It may be possible to rewrite in cleaner code. But I minimise changes to backport this PR to 1.9 branch.
Avoid unnecessary allocation.
Avoid unnecessary allocation. (cherry picked from commit 0fd55eb)
Avoid unnecessary allocation. (cherry picked from commit 0fd55eb)
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [github.com/go-sql-driver/mysql](https://github.com/go-sql-driver/mysql) | require | patch | `v1.9.2` -> `v1.9.3` | --- ### Release Notes <details> <summary>go-sql-driver/mysql (github.com/go-sql-driver/mysql)</summary> ### [`v1.9.3`](https://github.com/go-sql-driver/mysql/releases/tag/v1.9.3) [Compare Source](go-sql-driver/mysql@v1.9.2...v1.9.3) #### What's Changed - \[1.9] test stability improvement. by [@​methane](https://github.com/methane) in go-sql-driver/mysql#1699 - \[1.9] Transaction Commit/Rollback returns conn's cached error by [@​methane](https://github.com/methane) in go-sql-driver/mysql#1702 - backport benchmark_test by [@​methane](https://github.com/methane) in go-sql-driver/mysql#1706 - \[1.9] optimize readPacket ([#​1705](go-sql-driver/mysql#1705)) by [@​methane](https://github.com/methane) in go-sql-driver/mysql#1707 - \[1.9] fix PING on compressed connections by [@​methane](https://github.com/methane) in go-sql-driver/mysql#1723 - release v1.9.3 by [@​methane](https://github.com/methane) in go-sql-driver/mysql#1725 **Full Changelog**: go-sql-driver/mysql@v1.9.2...v1.9.3 </details> --- ### Configuration 📅 **Schedule**: Branch creation - Between 12:00 AM and 03:59 AM ( * 0-3 * * * ) (UTC), Automerge - Between 12:00 AM and 03:59 AM ( * 0-3 * * * ) (UTC). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MC40OC40IiwidXBkYXRlZEluVmVyIjoiNDAuNDguNCIsInRhcmdldEJyYW5jaCI6ImZvcmdlam8iLCJsYWJlbHMiOlsiZGVwZW5kZW5jeS11cGdyYWRlIiwidGVzdC9ub3QtbmVlZGVkIl19--> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8186 Reviewed-by: Earl Warren <[email protected]> Co-authored-by: Renovate Bot <[email protected]> Co-committed-by: Renovate Bot <[email protected]>
Description
Avoid unnecessary allocation.
Checklist
Summary by CodeRabbit
New Features
Refactor