Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .jules/bolt.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,7 @@
## 2026-05-15 - [Thread-Safe Compiled Scan Plans via Binding]
**Learning:** Moving reflection-heavy logic into closures (pre-compiled scan plans) can significantly reduce per-row overhead. However, when these plans are cached and shared across goroutines, any mutable state (like scan closures or index lists) must be isolated per-query. By introducing a separate 'boundRowScanPlan' created via a 'bind' method at query runtime, we achieved optimized, closure-based scanning while maintaining absolute thread safety and avoiding data corruption.
**Action:** When implementing closure-based optimizations for cached metadata objects, always separate the static 'plan' from the runtime 'bound' state to ensure thread safety and prevent concurrent query interference.

## 2026-05-23 - [Hot Scanning Loop Optimization via Kind Range Checks]
**Learning:** Even with pre-compiled scan plans, `reflect.Value.Kind()` checks and `switch` statements in the hot scanning loop (executed per column, per row) can add measurable overhead. Fast-pathing the most common database type (`reflect.Int64`) and using ordered `if/else` range checks for remaining types can reduce branch mispredictions and skip redundant overflow checks. Additionally, avoiding string allocations for cache keys in frequently called functions like `newRowScanPlanForColumns` provides a "free" win for point lookups and aggregations.
**Action:** Always identify and fast-path the most likely type in performance-critical loops. Use range checks on enums (like `reflect.Kind`) when safe to simplify branching logic. Avoid `strings.Join` or similar allocations for cache keys when inputs are singular.
36 changes: 27 additions & 9 deletions pkg/rain/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -369,18 +369,23 @@ func scanDirectRow(target reflect.Value, plan *rowScanPlan, scanned []any) error
} else {
field = target.Field(col.index0)
}
switch field.Kind() {
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
kind := field.Kind()
// OPTIMIZATION: Fast-path Int64 (the most common DB int type) and use range
// checks for other integer types to minimize branch overhead and avoid
// redundant overflow checks for the common path.
if kind == reflect.Int64 {
field.SetInt(v.Int64)
} else if kind >= reflect.Int && kind <= reflect.Int32 {
if field.OverflowInt(v.Int64) {
return fmt.Errorf("rain: value %d overflows field %s", v.Int64, field.Type())
}
field.SetInt(v.Int64)
case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
} else if kind >= reflect.Uint && kind <= reflect.Uint64 {
Comment on lines +378 to +383
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Implicit reliance on reflect.Kind numeric ordering

The range checks kind >= reflect.Int && kind <= reflect.Int32 and kind >= reflect.Uint && kind <= reflect.Uint64 are correct today (Int=2…Int32=5, Uint=7…Uint64=11 are contiguous and stable since Go 1), but this is an undocumented contract. If Go ever inserts a new Kind value inside one of those ranges, the branch would silently mismatch — e.g. a hypothetical reflect.Int128 between 5 and 6 would fall into the Int32 branch and hit SetInt without an overflow check. A comment citing the specific iota values being relied upon (e.g. // Int=2..Int32=5, Int64=6, Uint=7..Uint64=11) would make the assumption self-documenting and easier to audit in future Go upgrades.

Prompt To Fix With AI
This is a comment left during a code review.
Path: pkg/rain/model.go
Line: 378-383

Comment:
**Implicit reliance on `reflect.Kind` numeric ordering**

The range checks `kind >= reflect.Int && kind <= reflect.Int32` and `kind >= reflect.Uint && kind <= reflect.Uint64` are correct today (Int=2…Int32=5, Uint=7…Uint64=11 are contiguous and stable since Go 1), but this is an undocumented contract. If Go ever inserts a new Kind value inside one of those ranges, the branch would silently mismatch — e.g. a hypothetical `reflect.Int128` between 5 and 6 would fall into the Int32 branch and hit `SetInt` without an overflow check. A comment citing the specific iota values being relied upon (e.g. `// Int=2..Int32=5, Int64=6, Uint=7..Uint64=11`) would make the assumption self-documenting and easier to audit in future Go upgrades.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Codex

if v.Int64 < 0 || field.OverflowUint(uint64(v.Int64)) {
return fmt.Errorf("rain: value %d overflows field %s", v.Int64, field.Type())
}
field.SetUint(uint64(v.Int64))
default:
} else {
if err := assignRawValueToField(field, v.Int64); err != nil {
return err
}
Expand All @@ -407,18 +412,21 @@ func scanDirectRow(target reflect.Value, plan *rowScanPlan, scanned []any) error
field.Set(reflect.New(field.Type().Elem()))
}
field = field.Elem()
switch field.Kind() {
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
kind := field.Kind()
// OPTIMIZATION: Fast-path Int64 and use range checks to reduce overhead in the hot loop.
if kind == reflect.Int64 {
field.SetInt(v.Int64)
} else if kind >= reflect.Int && kind <= reflect.Int32 {
if field.OverflowInt(v.Int64) {
return fmt.Errorf("rain: value %d overflows field %s", v.Int64, field.Type())
}
field.SetInt(v.Int64)
case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
} else if kind >= reflect.Uint && kind <= reflect.Uint64 {
if v.Int64 < 0 || field.OverflowUint(uint64(v.Int64)) {
return fmt.Errorf("rain: value %d overflows field %s", v.Int64, field.Type())
}
field.SetUint(uint64(v.Int64))
default:
} else {
if err := assignRawValueToField(field, v.Int64); err != nil {
return err
}
Expand Down Expand Up @@ -737,7 +745,17 @@ func sliceElementStructType(elemType reflect.Type) (reflect.Type, bool, error) {
}

func newRowScanPlanForColumns(cols []string, modelType reflect.Type, table *schema.TableDef) (*rowScanPlan, error) {
columnKey := strings.Join(cols, "\x00")
var columnKey string
// OPTIMIZATION: Avoid strings.Join for the common single-column scan (Count/point lookups).
switch len(cols) {
case 0:
columnKey = ""
case 1:
columnKey = cols[0]
default:
columnKey = strings.Join(cols, "\x00")
}

key := rowScanPlanKey{
modelType: modelType,
columns: columnKey,
Expand Down