Skip to content

perf(replace): reduce allocations and vector copies in REPLACE INTO hot path - #24389

Merged
mergify[bot] merged 6 commits into
matrixorigin:mainfrom
ck89119:replace-opt-p0
May 15, 2026
Merged

perf(replace): reduce allocations and vector copies in REPLACE INTO hot path#24389
mergify[bot] merged 6 commits into
matrixorigin:mainfrom
ck89119:replace-opt-p0

Conversation

@ck89119

@ck89119 ck89119 commented May 14, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #23946

What this PR does / why we need it:

Three targeted optimizations to reduce overhead in the REPLACE INTO execution path (DedupJoin → LockOp → MultiUpdate):

1. Cache hashmap iterator in DedupJoin probe loop (dedupjoin/join.go)

  • ctr.mp.NewIterator() was called on every batch; now cached in ctr.cachedItr and reused across batches, avoiding repeated allocation.

2. Reduce vector copies in DedupJoin finalize (dedupjoin/join.go)

  • Non-capture build columns: transfer vector ownership (bat.Vecs[rp.Pos] = nil) instead of GetUnionAllFunction full copy.
  • Capture columns with no matched rows: emit a pre-filled NULL vector via AppendMultiFixed instead of copying from capturedVecs.
  • Replace per-row UnionOne loop with unionSelsByBatch, which groups selections by build-batch index and issues one Union call per group (with a ≤16-sels fast path to avoid grouping overhead for small inputs).

3. Reduce lock and insert overhead (lockop/lock_op.go, multi_update/insert.go, multi_update/types.go)

  • dedupLockRows: replace map-based dedup with sort + adjacent-compare dedup, eliminating string allocations per lock row.
  • Reuse types.NewPacker() across Prepare calls instead of allocating a new one each time.
  • insert_table: detect contiguous InsertCols mappings at init time (isContiguous); when true, point a cached refBatch at input vectors directly (zero-copy) instead of UnionBatch-copying into insertBatch.

Benchmark (YCSB REPLACE INTO, standalone MO, 100k rows, batchsize=1000):

threads main this PR vs main
32 1982.85 ops/s 2154.52 ops/s +8.66%
64 1924.70 ops/s 2042.75 ops/s +6.14%

ck89119 and others added 3 commits May 14, 2026 14:29
- Narrow OldColCaptureList to only downstream-required columns (RowID,
  PK, index keys) instead of capturing every main-table column. This
  reduces initCaptureBuffers pre-allocation, probe-side Copy calls, and
  finalize UnionBatch work proportionally to (total_cols - needed_cols).
- Cache hashmap iterator in DedupJoin container to avoid per-batch
  allocation of 4 internal slices (values/zValues/keys/strHashStates).
- Pre-size maps in bind_replace plan generation to eliminate rehashing.
- Reuse Packer in LockOp.Prepare instead of allocating a new one on
  every pipeline reset.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- insert_table: skip UnionBatch copy when InsertCols form a contiguous
  mapping; build a zero-copy reference batch sharing input vectors.
- LockOp: deduplicate lock rows (by encoded key) before calling
  lockService.Lock, avoiding redundant lock requests and redundant
  hasNewVersionInRange checks on identical PK values.
- DedupJoin finalize: replace per-row UnionOne loop with batch-grouped
  vector.Union calls (unionSelsByBatch), reducing vector grow/realloc.
- DedupJoin finalize (matched==0): transfer build-batch vector ownership
  to output instead of copying; emit pre-filled NULL for capture columns
  when no probe hit occurred.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ck89119
ck89119 requested review from aunjgr and ouyuanning as code owners May 14, 2026 06:31
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@mergify mergify Bot added the queued label May 15, 2026
@mergify

mergify Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • Entered queue2026-05-15 05:17 UTC · Rule: main
  • Checks passed · in-place
  • Merged2026-05-15 06:17 UTC · at 8fc3fb1dab9926eb8c48eaac3aad30cad9335a75 · squash

This pull request spent 1 hour 42 seconds in the queue, including 1 hour 3 seconds running CI.

Required conditions to merge
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
  • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • github-review-decision = APPROVED [🛡 GitHub branch protection]
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-neutral = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-skipped = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI / SCA Test on Ubuntu/x86
    • check-neutral = Matrixone CI / SCA Test on Ubuntu/x86
    • check-skipped = Matrixone CI / SCA Test on Ubuntu/x86
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI / UT Test on Ubuntu/x86
    • check-neutral = Matrixone CI / UT Test on Ubuntu/x86
    • check-skipped = Matrixone CI / UT Test on Ubuntu/x86
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-neutral = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-skipped = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Utils CI / Coverage
    • check-neutral = Matrixone Utils CI / Coverage
    • check-skipped = Matrixone Utils CI / Coverage

@mergify
mergify Bot merged commit 20346ca into matrixorigin:main May 15, 2026
23 of 24 checks passed
@mergify mergify Bot removed the queued label May 15, 2026
@ck89119
ck89119 deleted the replace-opt-p0 branch May 25, 2026 06:49
mergify Bot pushed a commit that referenced this pull request May 25, 2026
…24564)

Fixes a correctness regression introduced by #24389 in the DedupJoin `matched.Count()==0` fast path (`pkg/sql/colexec/dedupjoin/join.go`).

That fast path transfers ownership of build-batch vectors to the output (steal the vector, then `bat.Vecs[rp.Pos] = nil`), which assumes each non-capture build `ResultPos` is unique. But `constructDedupJoin` copies `node.ProjectList` into `ap.Result` without dedup, and the REPLACE planner can alias multiple projections onto the same build column, so a non-capture build position may be referenced more than once. With duplicates, the first projection steals the vector and the second reads `nil`, corrupting the output batch / panicking downstream (`vector.(*Vector).Size`).

**Fix:** count references per non-capture build position before the loop and transfer ownership only when a position is referenced exactly once; otherwise copy via `GetUnionAllFunction` so every reference gets its own valid vector.

**Test:** adds `TestDedupJoinFinalizeMatchedZero_DuplicateBuildPos`, which projects the same build column twice in the `matched==0` path. It panics on the pre-fix code and passes with the fix.

Approved by: @aunjgr
mergify Bot pushed a commit that referenced this pull request May 25, 2026
…ot path (cherry-pick #24389) (#24560)

Cherry-pick of #24389 to 4.0-dev.

Three targeted optimizations to reduce overhead in the REPLACE INTO execution path (DedupJoin → LockOp → MultiUpdate):

**1. Cache hashmap iterator in DedupJoin probe loop** (`dedupjoin/join.go`)
- `ctr.mp.NewIterator()` was called on every batch; now cached in `ctr.cachedItr` and reused across batches, avoiding repeated allocation.

**2. Reduce vector copies in DedupJoin finalize** (`dedupjoin/join.go`)
- Non-capture build columns: transfer vector ownership (`bat.Vecs[rp.Pos] = nil`) instead of `GetUnionAllFunction` full copy.
- Capture columns with no matched rows: emit a pre-filled NULL vector via `AppendMultiFixed` instead of copying from `capturedVecs`.
- Replace per-row `UnionOne` loop with `unionSelsByBatch`, which groups selections by build-batch index and issues one `Union` call per group (with a ≤16-sels fast path to avoid grouping overhead for small inputs).

**3. Reduce lock and insert overhead** (`lockop/lock_op.go`, `multi_update/insert.go`, `multi_update/types.go`)
- `dedupLockRows`: replace map-based dedup with sort + adjacent-compare dedup, eliminating string allocations per lock row.
- Reuse `types.NewPacker()` across `Prepare` calls instead of allocating a new one each time.
- `insert_table`: detect contiguous `InsertCols` mappings at init time (`isContiguous`); when true, point a cached `refBatch` at input vectors directly (zero-copy) instead of `UnionBatch`-copying into `insertBatch`.

**Benchmark (YCSB REPLACE INTO, standalone MO, 100k rows, batchsize=1000):**

| threads | main | this PR |  vs main |
|---------|------|---------|----------|
| 32 | 1982.85 ops/s | 2154.52 ops/s | **+8.66%** |
| 64 | 1924.70 ops/s | 2042.75 ops/s | **+6.14%** |

Approved by: @XuPeng-SH, @aunjgr, @ouyuanning
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/enhancement size/XL Denotes a PR that changes [1000, 1999] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants