Skip to content

Eliminate CGO overhead in row scanning hot path#141

Merged
taniabogatsch merged 2 commits into
duckdb:mainfrom
krleonid:eliminate-cgo-scan-overhead-main
May 28, 2026
Merged

Eliminate CGO overhead in row scanning hot path#141
taniabogatsch merged 2 commits into
duckdb:mainfrom
krleonid:eliminate-cgo-scan-overhead-main

Conversation

@krleonid
Copy link
Copy Markdown

@krleonid krleonid commented May 28, 2026

Summary

  • Replace per-cell CGO calls in getNull and getBytes with pure Go pointer arithmetic, reducing ~500K CGO boundary crossings per 1000-row × 250-column query to zero
  • Bypass verifyAndRewriteColIdx in rows.Next by calling getFn directly (no bounds/projection check needed on the read path)

Measured improvement

Single query, 1000 rows × 250 columns, no concurrency:

Metric Before After
Avg scan time 16.4ms 8.1ms
Per-row scan 16.4µs 8.1µs

Under 90% CPU pressure (10 concurrent workers):

Metric Before After
Avg scan time 432ms 255ms
Max scan time 1.19s 622ms

Why this matters

Each CGO call pins a goroutine to an OS thread and creates a preemption point. Under CPU contention (70-90%), the OS scheduler delays resumption at each crossing. With 500K crossings per query, even rare preemption events accumulate into multi-second scan stalls in production (observed 1-3s spikes on 32-core pods at 70%+ CPU).

After this change, the scan loop is pure Go — no thread pinning, no preemption points between cells. The goroutine runs uninterrupted through all 250K cells per query.

Changes

  • vector_getters.go: getNull — inline bit manipulation instead of mapping.ValidityMaskValueIsValid CGO call. The validity mask is a uint64 bitfield already accessible via vec.maskPtr; we just read and bit-test it directly.
  • vector_getters.go: getBytes — read duckdb_string_t layout directly (uint32 length at offset 0, inlined data at offset 4 for short strings, pointer at offset 8 for long strings) instead of mapping.StringTData which made 2 CGO calls (C.duckdb_string_t_length + C.duckdb_string_t_data) plus C.GoBytes.
  • rows.go: Next() — call getFn directly on the column vector, skipping GetValue/verifyAndRewriteColIdx bounds checking (indices are always valid in the rows path).

Safety

  • Memory lifetime: getBytes uses strings.Clone to copy string data to the Go heap before the chunk can be freed (same guarantee as before).
  • Alignment: duckdb_string_t is 16-byte aligned in DuckDB column storage. Reads at offset 0 (uint32) and offset 8 (pointer) are naturally aligned.
  • Validity mask layout: matches DuckDB's duckdb_go_bindings_is_valid implementation exactly (mask[index/64] & (1 << index%64)).

Test plan

  • go test ./... passes (full test suite including TestTypes, TestBlob, TestBit, TestList, TestMap, TestEnumNullValues, TestEmbeddedNulls, etc.)
  • Benchmarked with 1000 rows × 250 columns (142 STRUCT columns, VARCHAR-heavy schema)
  • Verified under CPU pressure with 10 concurrent workers on 65 open connections

🤖 Generated with Claude Code

Replace per-cell CGO calls in getNull and getBytes with pure Go pointer
arithmetic, reducing ~500K CGO boundary crossings per 1000-row × 250-column
query to zero. Bypass verifyAndRewriteColIdx in rows.Next by calling getFn
directly.

Measured improvement (single query, 1000 rows × 250 cols):
- Scan time: 16.4ms → 8.1ms (2× faster)
- Under 90% CPU pressure: max scan 1.2s → 0.9s

Changes:
- vector_getters.go: getNull uses inline bit manipulation instead of
  mapping.ValidityMaskValueIsValid CGO call
- vector_getters.go: getBytes reads duckdb_string_t layout directly
  instead of mapping.StringTData (2 CGO calls + C.GoBytes)
- rows.go: Next() calls getFn directly, skipping GetValue/verifyAndRewriteColIdx

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@taniabogatsch taniabogatsch left a comment

Choose a reason for hiding this comment

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

LGTM - just had two nits/notes on comments.

Comment thread vector_getters.go Outdated
Comment thread vector_getters.go
- Use "entry" instead of "word" in getNull comments to match DuckDB convention
- Add note about INLINE_LENGTH not being exposed in the C API

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@taniabogatsch taniabogatsch merged commit 28a82e9 into duckdb:main May 28, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants