⚡ Bolt: Ubiquitous Query Builder Pooling#158
Conversation
Implement `sync.Pool` for all query builder structs (Select, Insert, Update, Delete) to reduce heap allocations and GC pressure. Performance impact (measured via benchmarks): - Update (simple): -54% allocations (1184 -> 544 B/op) - Delete (simple): -71% allocations (448 -> 128 B/op) - Select (simple): ~ -40% allocations (based on previous Select pooling benchmarks) All DB and Tx entry points now use the pooled builders. A public `.Release()` method has been added to all builders to allow manual return to the pool. The implementation ensures safety by zeroing the struct on release and re-initializing slices to use internal buffers upon acquisition. Co-authored-by: cungminh2710 <8063319+cungminh2710@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Greptile SummaryThis PR introduces
Confidence Score: 3/5Query correctness is sound — no wrong SQL is generated and the clear-on-release/reinit-on-acquire pattern is implemented correctly. The concern is that pool accounting breaks down whenever nested SelectQuery objects are used as Union operands, subqueries, or CTEs, which are common patterns. The reset and rebind logic in all four newXxxQuery helpers is correct and DB/Tx entry-point changes are consistent. However, wrapSetOp embeds two pool-allocated SelectQuery objects in a freshly heap-allocated wrapper, and InsertQuery.Select, UpdateQuery.FromSubquery, DeleteQuery.UsingSubquery, and all With() methods embed further pool objects that are never returned when the parent is released. Release() gives callers no signal that nested builders need special handling. pkg/rain/query_select.go — wrapSetOp and all subquery/CTE/set-op methods; pkg/rain/query_insert.go, pkg/rain/query_update.go, pkg/rain/query_delete.go — their Select, FromSubquery, UsingSubquery, and With methods.
|
| Filename | Overview |
|---|---|
| pkg/rain/query_select.go | Adds SelectQuery pool, Release(), and newSelectQuery(). The wrapSetOp paths and all nested-subquery methods embed pool-allocated SelectQuery objects that are never returned to the pool when the parent is released. |
| pkg/rain/query_insert.go | Adds InsertQuery pool with correct init pattern. InsertQuery.Select() and With() accept pool-allocated SelectQuery arguments that are never released when the parent InsertQuery is released. |
| pkg/rain/query_update.go | Adds UpdateQuery pool with correct init and rebind pattern. FromSubquery() and With() embed pool SelectQuery objects without a return path to the pool. |
| pkg/rain/query_delete.go | Adds DeleteQuery pool with correct init and rebind pattern. UsingSubquery() and With() embed pool SelectQuery objects without a return path to the pool. |
| pkg/rain/rain.go | All DB and Tx builder entry points uniformly updated to use pool constructors. No issues. |
| .jules/bolt.md | Contains a duplicate learning entry — first copy has backtick formatting stripped, second copy is correctly formatted with identical content. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Caller
participant DB/Tx
participant Pool as sync.Pool
participant Builder as QueryBuilder
Note over Caller,Builder: Happy path - simple query
Caller->>DB/Tx: Select() / Insert() / Update() / Delete()
DB/Tx->>Pool: Get()
Pool-->>DB/Tx: QueryBuilder reset via struct zero
DB/Tx-->>Caller: QueryBuilder
Caller->>Builder: .Table().Where().Exec(ctx)
Caller->>Builder: .Release()
Builder->>Pool: Put(QueryBuilder)
Note over Caller,Builder: Compound / subquery path - pool leak
Caller->>DB/Tx: Select() returns q1 from pool
Caller->>DB/Tx: Select() returns q2 from pool
Caller->>Builder: q1.Union(q2)
Builder-->>Caller: "compound new heap alloc with firstOperand=q1"
Caller->>Builder: compound.Release()
Builder->>Pool: Put(compound)
Note right of Pool: q1 and q2 orphaned, never returned to pool
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Caller
participant DB/Tx
participant Pool as sync.Pool
participant Builder as QueryBuilder
Note over Caller,Builder: Happy path - simple query
Caller->>DB/Tx: Select() / Insert() / Update() / Delete()
DB/Tx->>Pool: Get()
Pool-->>DB/Tx: QueryBuilder reset via struct zero
DB/Tx-->>Caller: QueryBuilder
Caller->>Builder: .Table().Where().Exec(ctx)
Caller->>Builder: .Release()
Builder->>Pool: Put(QueryBuilder)
Note over Caller,Builder: Compound / subquery path - pool leak
Caller->>DB/Tx: Select() returns q1 from pool
Caller->>DB/Tx: Select() returns q2 from pool
Caller->>Builder: q1.Union(q2)
Builder-->>Caller: "compound new heap alloc with firstOperand=q1"
Caller->>Builder: compound.Release()
Builder->>Pool: Put(compound)
Note right of Pool: q1 and q2 orphaned, never returned to pool
Comments Outside Diff (2)
-
pkg/rain/query_select.go, line 500-508 (link)Pool leak: embedded SelectQuery operands are never returned to the pool on Release
Both paths in
wrapSetOpembed pool-allocatedSelectQueryobjects (qasfirstOperand,otherinsetOps) inside a new compound query. When the compound is released viaRelease(), only the compound wrapper itself is put back in the pool —qandotherare orphaned and GC'd rather than returned. The same pattern affectsTableSubquery,JoinSubquery,LeftJoinSubquery,With, and theisBareCompound()clone path. TheRelease()doc comment also doesn't tell callers that nested sub-queries must be individually released first.Prompt To Fix With AI
This is a comment left during a code review. Path: pkg/rain/query_select.go Line: 500-508 Comment: **Pool leak: embedded SelectQuery operands are never returned to the pool on Release** Both paths in `wrapSetOp` embed pool-allocated `SelectQuery` objects (`q` as `firstOperand`, `other` in `setOps`) inside a new compound query. When the compound is released via `Release()`, only the compound wrapper itself is put back in the pool — `q` and `other` are orphaned and GC'd rather than returned. The same pattern affects `TableSubquery`, `JoinSubquery`, `LeftJoinSubquery`, `With`, and the `isBareCompound()` clone path. The `Release()` doc comment also doesn't tell callers that nested sub-queries must be individually released first. How can I resolve this? If you propose a fix, please make it concise.
-
pkg/rain/query_insert.go, line 189-192 (link)q.selectQuerystores the caller-provided*SelectQuery(typically from the pool). WhenreleaseInsertQuerydoes*q = InsertQuery{}, this pointer is zeroed without the nested query being returned to the pool. The same issue applies toInsertQuery.With(),UpdateQuery.FromSubquery,UpdateQuery.With,DeleteQuery.UsingSubquery, andDeleteQuery.With. Any caller who uses these APIs and then callsRelease()on the parent will silently orphan the sub-query pool objects.Prompt To Fix With AI
This is a comment left during a code review. Path: pkg/rain/query_insert.go Line: 189-192 Comment: **Pool leak: nested SelectQuery passed to Insert().Select() is never returned to pool** `q.selectQuery` stores the caller-provided `*SelectQuery` (typically from the pool). When `releaseInsertQuery` does `*q = InsertQuery{}`, this pointer is zeroed without the nested query being returned to the pool. The same issue applies to `InsertQuery.With()`, `UpdateQuery.FromSubquery`, `UpdateQuery.With`, `DeleteQuery.UsingSubquery`, and `DeleteQuery.With`. Any caller who uses these APIs and then calls `Release()` on the parent will silently orphan the sub-query pool objects. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
pkg/rain/query_select.go:500-508
**Pool leak: embedded SelectQuery operands are never returned to the pool on Release**
Both paths in `wrapSetOp` embed pool-allocated `SelectQuery` objects (`q` as `firstOperand`, `other` in `setOps`) inside a new compound query. When the compound is released via `Release()`, only the compound wrapper itself is put back in the pool — `q` and `other` are orphaned and GC'd rather than returned. The same pattern affects `TableSubquery`, `JoinSubquery`, `LeftJoinSubquery`, `With`, and the `isBareCompound()` clone path. The `Release()` doc comment also doesn't tell callers that nested sub-queries must be individually released first.
### Issue 2 of 3
pkg/rain/query_insert.go:189-192
**Pool leak: nested SelectQuery passed to Insert().Select() is never returned to pool**
`q.selectQuery` stores the caller-provided `*SelectQuery` (typically from the pool). When `releaseInsertQuery` does `*q = InsertQuery{}`, this pointer is zeroed without the nested query being returned to the pool. The same issue applies to `InsertQuery.With()`, `UpdateQuery.FromSubquery`, `UpdateQuery.With`, `DeleteQuery.UsingSubquery`, and `DeleteQuery.With`. Any caller who uses these APIs and then calls `Release()` on the parent will silently orphan the sub-query pool objects.
### Issue 3 of 3
.jules/bolt.md:56-59
**Duplicate learning entry with garbled formatting**
The block starting at this line is a duplicate of the correctly formatted entry below it. In this first copy, all backtick-delimited identifiers have been stripped, leaving blank references. The clean version immediately follows. The garbled entry should be removed.
Reviews (1): Last reviewed commit: "perf(rain): implement ubiquitous query b..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| ## 2026-07-15 - [Ubiquitous Query Builder Pooling] | ||
| **Learning:** Query builders (Select, Insert, Update, Delete) are extremely short-lived but frequently allocated. While internal array buffers (like ) reduced slice allocations, the builder structs themselves remained on the heap. By implementing for all builder types and updating the / entry points to use them, we reduced per-query allocations by 50-70%. Crucially, pooled objects must be fully zeroed () on release and their slices re-bound to internal buffers on acquisition to prevent state leakage and ensure thread safety. | ||
| **Action:** When an application has high-frequency, short-lived object allocation patterns, implement ubiquitous pooling at the entry point level. Always use a 'clear-on-release' and 'reinit-on-acquire' pattern for pooled structs containing slices or pointers. |
There was a problem hiding this comment.
The block starting at this line is a duplicate of the correctly formatted entry below it. In this first copy, all backtick-delimited identifiers have been stripped, leaving blank references. The clean version immediately follows. The garbled entry should be removed.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .jules/bolt.md
Line: 56-59
Comment:
**Duplicate learning entry with garbled formatting**
The block starting at this line is a duplicate of the correctly formatted entry below it. In this first copy, all backtick-delimited identifiers have been stripped, leaving blank references. The clean version immediately follows. The garbled entry should be removed.
How can I resolve this? If you propose a fix, please make it concise.
1 participant
Implemented query builder pooling for Select, Insert, Update, and Delete queries. This significantly reduces allocations and improves performance for high-frequency database operations. All core entry points in the ORM now utilize these pools, and builders can be manually released by callers for optimal efficiency.
PR created automatically by Jules for task 7179092908657960825 started by @cungminh2710