Optimization: statements reuse previous column name #1709
Conversation
WalkthroughThis set of changes refactors the Changes
Sequence Diagram(s)sequenceDiagram
participant Benchmark as BenchmarkReceiveMetadata
participant DB as Database
participant Stmt as PreparedStatement
Benchmark->>DB: SetMaxIdleConns(0), SetMaxIdleConns(1)
Benchmark->>DB: Prepare statement
DB-->>Benchmark: PreparedStatement
Benchmark->>Stmt: Execute query in loop (b.Loop())
Stmt->>DB: Query execution
DB-->>Stmt: Rows
Stmt->>Benchmark: Scan row into slices
Benchmark->>Stmt: Close rows
Benchmark->>Stmt: Close statement (deferred)
sequenceDiagram
participant Caller as Internal Caller
participant Conn as mysqlConn
participant Columns as Columns Slice
Caller->>Conn: readColumns(count, oldColumns)
Conn->>Columns: Compare old/new metadata strings
alt Strings match
Conn->>Columns: Reuse string from oldColumns
else Strings differ
Conn->>Columns: Allocate new string
end
Conn-->>Caller: Return columns slice
Possibly related PRs
Poem
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
benchmark_test.go (1)
482-483: Simplify connection pool initialization.Setting
MaxIdleConnsto 0 and then immediately to 1 is redundant. You could directly set it to 1.- db.SetMaxIdleConns(0) - db.SetMaxIdleConns(1) + db.SetMaxIdleConns(1)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
benchmark_test.go(1 hunks)connection.go(2 hunks)packets.go(3 hunks)rows.go(2 hunks)statement.go(2 hunks)
🧰 Additional context used
🪛 golangci-lint (1.64.8)
benchmark_test.go
500-500: stdversion: testing.Loop requires go1.24 or later (file is go1.22)
(govet)
🪛 GitHub Check: test (ubuntu-latest, 1.23, 9.0)
benchmark_test.go
[failure] 500-500:
b.Loop undefined (type *testing.B has no field or method Loop)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: test (windows-latest, 1.24, mariadb-11.4)
- GitHub Check: test (ubuntu-latest, 1.24, 8.0)
- GitHub Check: test (windows-latest, 1.24, mariadb-11.1)
- GitHub Check: test (ubuntu-latest, 1.24, mariadb-11.2)
- GitHub Check: test (windows-latest, 1.24, mariadb-10.6)
- GitHub Check: test (windows-latest, 1.24, mariadb-10.11)
- GitHub Check: test (ubuntu-latest, 1.24, mariadb-10.6)
- GitHub Check: test (windows-latest, 1.24, 9.0)
- GitHub Check: test (ubuntu-latest, 1.24, mariadb-10.5)
- GitHub Check: test (ubuntu-latest, 1.24, mariadb-11.4)
- GitHub Check: test (macos-latest, 1.24, 9.0)
- GitHub Check: test (windows-latest, 1.24, 8.4)
- GitHub Check: test (ubuntu-latest, 1.24, mariadb-11.1)
- GitHub Check: test (ubuntu-latest, 1.24, mariadb-10.11)
- GitHub Check: test (macos-latest, 1.24, mariadb-11.1)
- GitHub Check: test (macos-latest, 1.24, 5.7)
- GitHub Check: test (macos-latest, 1.24, 8.4)
- GitHub Check: test (ubuntu-latest, 1.24, 5.7)
- GitHub Check: test (ubuntu-latest, 1.24, 8.4)
- GitHub Check: test (ubuntu-latest, 1.24, 9.0)
🔇 Additional comments (13)
connection.go (2)
234-234: UpdatedreadColumnscall with the new parameterThis change adapts the call to
readColumnsto include the newnilparameter, which aligns with the updated method signature inpackets.go. This enables the string reuse optimization path when applicable.
451-451: UpdatedreadColumnscall in query methodSimilar to the previous change, this updates the
readColumnscall in the query method to include thenilparameter. Since this is a new query without previous column metadata, passingnilis appropriate here.statement.go (2)
77-77: Passing existing columns for potential metadata reuseThis change passes the statement's existing columns (
stmt.columns) to thereadColumnsmethod, enabling reuse of column name strings when the metadata matches. This is a key part of the optimization, as prepared statements benefit the most from column metadata reuse.
128-128: Reusing column metadata in query executionSimilar to the previous change, this passes
stmt.columnstoreadColumnsduring query execution, allowing for reuse of column metadata when the structure matches. The optimization is particularly effective for prepared statements that are executed repeatedly.rows.go (2)
189-189: UpdatedreadColumnscall in NextResultSet for binary rowsThis change adapts the call to
readColumnsto include the newnilparameter in thebinaryRows.NextResultSetmethod. Passingnilis appropriate here as there's no previous column metadata to reuse in this context.
211-211: UpdatedreadColumnscall in NextResultSet for text rowsSimilar to the previous change, this updates the
readColumnscall in thetextRows.NextResultSetmethod to include thenilparameter, maintaining consistency with the updated method signature.packets.go (3)
705-709: Added string reuse optimization parameter toreadColumnsThe function signature has been updated to accept an additional
old []mysqlFieldparameter, which enables string reuse for column metadata. The check on line 707-709 ensures that we only attempt to reuse strings if the number of columns matches, which is a necessary precondition for the optimization.
737-742: Implemented tableName string reuseThis block implements the string reuse optimization for the column's
tableNamefield. If the old column metadata exists and has the same tableName as the one being read, the code reuses the existing string instead of allocating a new one. This is a memory optimization that reduces garbage collection pressure.
763-768: Implemented column name string reuseSimilar to the tableName optimization, this block implements string reuse for the column's
namefield. By reusing existing string objects when column names match, we avoid unnecessary allocations. This is particularly effective for prepared statements that are executed frequently with the same column structure.benchmark_test.go (4)
496-498: The benchmark structure looks good.The setup for benchmarking with
ReportAllocs()andResetTimer()is appropriate for measuring memory allocations, which aligns with the PR's goal of reducing allocations when handling statement metadata.
485-490: Good practice for scanning multiple columns.The approach of pre-allocating slices for values and pointers is an efficient way to scan rows with many columns. This setup supports the PR's goal of benchmarking metadata handling with minimal overhead.
501-509: Benchmark logic aligns with PR objectives.The benchmark correctly tests the core functionality targeted by this PR: optimizing statement metadata handling. By executing a query against a table with 1000 columns and measuring allocations, it effectively demonstrates the memory usage improvements from reusing column names.
500-500:❌ Incorrect review comment
Fix Go version compatibility issue with
b.Loop().The
Loop()method requires Go 1.24 or later, but according to static analysis, this file is using Go 1.22. This will cause a compilation error.- for b.Loop() { + for i := 0; i < b.N; i++ {The benchmark logic is valid, but the loop implementation needs to be compatible with the Go version used in the project.
🏁 Script executed:
#!/bin/bash # Check the Go version used in the project go versionLength of output: 44
No change needed:
b.Loop()is supported under Go 1.24.1
The project’s Go version is 1.24.1 (pergo version), so usingb.Loop()in benchmarks is fully compatible.Likely an incorrect or invalid review comment.
🧰 Tools
🪛 golangci-lint (1.64.8)
500-500: stdversion: testing.Loop requires go1.24 or later (file is go1.22)
(govet)
🪛 GitHub Check: test (ubuntu-latest, 1.23, 9.0)
[failure] 500-500:
b.Loop undefined (type *testing.B has no field or method Loop)
2 participants
Description
#1708 added
[]mysqlFieldcache to stmt. It was used only for MariaDB cached metadata.This commit allows MySQL to also benefit from the metadata cache. If the column names are the same as the cached metadata, it reuses them instead of allocating new strings.
Checklist
Summary by CodeRabbit
Refactor
Performance