perf: replace stdlib sort.Slice with slices pkg to eliminate heap escapes

[fengttt]

What

Escape-analysis-driven pass over stdlib sort. sort.Slice(s, less) boxes its slice argument into interface{}, which forces s escapes to heap (a per-call allocation — confirmed with -gcflags=-m) and uses reflection for element swaps. The generic slices.Sort / slices.SortFunc / slices.SortStableFunc do neither.

sort.Slice(x, func(i,j int) bool { return x[i] < x[j] })   // x escapes to heap
slices.Sort(x)                                             // x does not escape

Changes

Converted ~104 stdlib sort.Slice / sort.SliceStable sites across 66 files:

• ordered element slices → slices.Sort
• keyed / descending / multi-key → slices.SortFunc with cmp.Compare (cmp.Compare, never a-b, is overflow-safe)
• sort.SliceStable → slices.SortStableFunc (stability preserved)
• index-permutation sorts → the element values (a,b) index the target array
• boolean lessFn predicates → two-call 3-way comparator

Also converted the one slice-based 1-arg sort.Sort (disttae/cache Columns, a []catalog.Column named type), which escapes for the same reason.

Deliberately left unchanged

• sort.Sort on pointer receivers (*groupSortKeys, *BatchWithVersion): a pointer fits in the interface word, so boxing does not allocate — no benefit.
• SkipCmd.Sort in logentry.go: its comparator has a side effect (swaps a parallel psns array during comparison), so slices.SortFunc's different call pattern would corrupt it. Tracked in Remove side-effecting sort comparator in SkipCmd.Sort (logentry.go) #25615.
• sort.Search / sort.Strings / sort.Float64s: not slice-boxing.

Verification

• go build ./... — clean
• go vet on all 66 touched packages — clean
• gofmt — clean
• targeted unit tests across taskservice, hakeeper, shardservice, geo, fulltext, tae/compute, tae/db/merge, tae/catalog, readutil, disttae/cache, bytejson, sql/plan, sql/plan/function, colexec/external, memoryengine — all pass

🤖 Generated with Claude Code

---

Refs #25639 (owner will close manually)
Refs #25646 (owner will close manually)
Refs #25702 (owner will close manually)

[qodo-code-review]

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 →

[gouhongshen]

Blocker: the two semantic-order-sensitive conversions pass non-strict comparators to slices.SortFunc. The API explicitly requires a strict weak ordering; returning a negative result for equal values makes cmp(x,x) < 0, so the claimed behavior preservation is outside the API contract and depends on the current sorting implementation. Since filter order is already demonstrated to affect query correctness, this cannot be merged as-is. Please either define an explicit, valid tie-breaker that captures the required semantic order, or leave these two sites on sort.Slice and address their pre-existing invalid comparators separately.

[LeftHandCold]

Two additional comparator-contract violations found while auditing the remaining conversions; these are the same root class as the existing requested changes.

[gouhongshen]

Additional findings from a re-audit of the current head:

• The two new P1 findings are inline below: the join-order comparator is non-transitive, and the IVF distance-bound folding can remove the first non-literal predicate before runtime validation.
• The last commit also expands this PR beyond a mechanical sort migration: it changes IVF predicate semantics and SQL goldens. Please either split that behavior fix or document it as an independently verified scope.
• The new numeric-bound tests cover literal bounds and order reversal, but not first-invalid/second-valid bounds, NULL/column/parameter bounds, or end-to-end mode=pre/mode=post preservation.

Existing stats/txn/zone-map threads are intentionally not duplicated. I added a follow-up to the existing snapshot thread for the remaining nil dereference path.

[Copilot]

Pull request overview

This PR is a broad performance-focused refactor across MatrixOne that replaces sort.Slice / sort.SliceStable (and one slice-based sort.Sort) with the generic slices package to reduce heap escapes and reflection overhead, while also tightening determinism/robustness around vector-distance predicate handling and filter ordering.

Changes:

• Replaced many sort.Slice / sort.SliceStable call sites with slices.Sort / slices.SortFunc / slices.SortStableFunc (often via cmp.Compare) to avoid slice boxing and improve escape behavior.
• Made planner filter ordering deterministic via stable, strict comparators, updating expected test outputs accordingly.
• Updated vector distance bound extraction to fold multiple same-side bounds into the tightest bound and added unit coverage for order-independence.

Reviewed changes

Copilot reviewed 70 out of 71 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test/distributed/cases/vector/vector_ivf_mode.sql	Clarifies intended invariant for multiple distance bounds (intersection, order-independent).
test/distributed/cases/optimizer/in_domain.result	Updates expected EXPLAIN filter order due to new deterministic sorting.
test/distributed/cases/optimizer/associative.result	Updates expected filter ordering in plan output.
test/distributed/cases/operator/is_not_operator.result	Updates expected error text driven by deterministic filter evaluation order.
pkg/vm/engine/tae/txn/txnimpl/table.go	Switches to slices.SortStableFunc for object ordering without sort.Slice boxing.
pkg/vm/engine/tae/logtail/snapshot.go	Replaces several sort.Slice uses with slices.SortFunc for deterministic TS ordering.
pkg/vm/engine/tae/logtail/handle.go	Uses slices.Sort for key ordering.
pkg/vm/engine/tae/logtail/ckp_reader.go	Uses slices.SortFunc + cmp.Compare for descending counts.
pkg/vm/engine/tae/logstore/driver/batchstoredriver/file.go	Uses slices.SortFunc for version ordering of log files.
pkg/vm/engine/tae/db/merge/statOverlap.go	Uses slices.SortFunc for overlap ratio ranking.
pkg/vm/engine/tae/db/merge/statLayerZero.go	Uses slices.SortFunc for min zone-map ordering.
pkg/vm/engine/tae/db/checkpoint/snapshot.go	Replaces sort.Slice with strict slices.SortFunc (including deterministic nil handling).
pkg/vm/engine/tae/db/checkpoint/replay.go	Uses slices.SortFunc for checkpoint file ordering.
pkg/vm/engine/tae/compute/compute.go	Migrates generic SortAndDedup to slices.SortFunc.
pkg/vm/engine/tae/common/intervals.go	Replaces interval sorting with slices/cmp.
pkg/vm/engine/tae/catalog/schema.go	Sorts sort-key defs with slices.SortFunc + cmp.
pkg/vm/engine/readutil/tombstone_data.go	Sorts Rowids via slices.SortFunc rather than sort.Slice.
pkg/vm/engine/memoryengine/shard.go	Sorts shard infos with slices.SortFunc + cmp.
pkg/vm/engine/memoryengine/shard_hash.go	Replaces several sort.Slice calls with slices.SortFunc.
pkg/vm/engine/disttae/txn.go	Uses slices.Sort / slices.SortFunc for deterministic workspace ordering.
pkg/vm/engine/disttae/tracked_partitions.go	Uses slices.SortFunc to evict oldest snapshot deterministically.
pkg/vm/engine/disttae/logtailreplay/get_flush_ts.go	Uses slices.SortFunc for time ordering.
pkg/vm/engine/disttae/local_disttae_datasource.go	Introduces strict ordering for zone-map init state + slices.SortFunc.
pkg/vm/engine/disttae/cache/catalog.go	Replaces sort.Sort (slice named type) with slices.SortFunc.
pkg/vectorindex/cuvs/cdc.go	Uses slices/cmp for chunk and output ordering.
pkg/util/debug/goroutine/analyze.go	Sorts groups by size with slices.SortFunc.
pkg/txn/storage/memorystorage/mem_handler.go	Uses slices.SortFunc for column/def ordering.
pkg/txn/client/operator.go	Sorts tracker info with slices.SortFunc + cmp.
pkg/tools/checkpointtool/checkpoint_reader.go	Uses slices.SortFunc for entry/account/table ordering.
pkg/taskservice/task_runner.go	Uses slices.SortFunc for retry queue ordering.
pkg/taskservice/mem_task_storage.go	Uses slices.SortFunc/cmp for deterministic in-memory query output ordering.
pkg/sql/schedule/placement.go	Replaces worker ordering with slices.SortFunc.
pkg/sql/plan/tools/helper.go	Uses slices.SortFunc to canonicalize var ref lists deterministically.
pkg/sql/plan/stats.go	Makes filter-cost ordering stable + strict via slices.SortStableFunc.
pkg/sql/plan/join_order.go	Replaces subtree/dimension ordering with slices.SortFunc.
pkg/sql/plan/function/func_builtin_jq.go	Uses slices.SortFunc for deterministic map key ordering.
pkg/sql/plan/function/func_binary.go	Uses slices.SortFunc for geometry interval ordering.
pkg/sql/plan/explain/explain_node.go	Uses slices.Sort for deterministic reporting arrays.
pkg/sql/plan/apply_indices_vector.go	Folds multiple distance bounds and adds helpers for literal extraction/merging.
pkg/sql/plan/apply_indices_vector_test.go	Updates/extends tests to assert tightest-bound folding and order-independence.
pkg/sql/compile/compile.go	Replaces several stable sorts with slices.SortStableFunc.
pkg/sql/colexec/table_function/processlist.go	Sorts sessions using slices.SortFunc.
pkg/sql/colexec/lockop/lock_op.go	Sorts lock rows with slices.SortFunc.
pkg/sql/colexec/external/hive_partition.go	Uses slices.SortFunc for deterministic file/prefix ordering.
pkg/sql/colexec/external/hive_partition_cache.go	Uses slices.SortFunc for deterministic dir entry ordering.
pkg/shardservice/scheduler_balance.go	Uses slices.SortFunc for replica-count ordering.
pkg/shardservice/runtime.go	Uses slices.SortFunc for deterministic CN ordering.
pkg/hakeeper/operator/builder.go	Uses slices.Sort for deterministic peer lists.
pkg/hakeeper/checkers/tnservice/parse.go	Uses slices.Sort to stabilize shard ID order.
pkg/hakeeper/checkers/tnservice/check.go	Uses slices.SortFunc + cmp across several selection orderings.
pkg/hakeeper/checkers/logservice/parse.go	Uses slices.Sort for replica ID ordering.
pkg/hakeeper/checkers/logservice/filter.go	Uses slices.SortFunc for store selection.
pkg/hakeeper/checkers/logservice/check.go	Replaces non-strict comparator with slices.SortFunc + cmp.
pkg/hakeeper/bootstrap/bootstrap.go	Uses slices.SortFunc + cmp for tick-based ordering.
pkg/geo/hull.go	Uses slices.SortFunc + cmp for coordinate ordering.
pkg/fulltext/fixedbytepool.go	Uses slices.SortFunc for LRU ordering.
pkg/frontend/rewrite_rule.go	Uses slices.SortStableFunc with strict comparator for rule cache ordering.
pkg/frontend/mysql_cmd_executor.go	Uses slices.SortFunc for SHOW output ordering.
pkg/frontend/logservice.go	Replaces several sort.Slice orderings with strict slices.SortFunc + cmp.
pkg/frontend/data_branch_privilege.go	Uses slices.Sort for deterministic ID lists.
pkg/frontend/compiler_context.go	Uses slices.SortFunc for UDF match ordering.
pkg/fileservice/s3_fs.go	Uses slices.SortFunc for IO entry ordering.
pkg/fileservice/qcloud_sdk.go	Uses slices.SortFunc for multipart part ordering.
pkg/fileservice/memory_fs.go	Uses slices.SortFunc for IO entry ordering.
pkg/fileservice/local_fs.go	Uses slices.SortFunc for IO entry ordering.
pkg/fileservice/local_etl_fs.go	Uses slices.SortFunc for IO entry ordering.
pkg/fileservice/aws_sdk_v2.go	Uses slices.SortFunc for multipart part ordering.
pkg/clusterservice/cluster.go	Uses slices.SortFunc to keep TN stores ordered by tick.
pkg/backup/types.go	Uses slices.SortFunc + cmp for meta type ordering.
pkg/backup/tae.go	Replaces meta file ordering with slices.SortFunc.
cmd/mo-service/debug.go	Uses slices.SortFunc + cmp for deterministic debug output ordering.

[aptend]

Requesting changes for the remaining correctness blockers on the current head:

1. The first distance bound is accepted without validating that it is a numeric literal. When the first matching upper or lower bound is a column, parameter, NULL, or another unsupported expression, mergeUpperBound / mergeLowerBound stores it in DistRange and removes the predicate from the residual filter list. The runtime reader then fails getLiteralFloat64 and returns without installing the bound, so the query can execute without that predicate. Please validate an unbounded side before storing it, preserve unsupported bounds as residual filters, and add first-invalid/second-valid tests in both orders.

2. The two join-order conversions use a non-transitive comparator. compareStats switches between selectivity and out-count within a 0.01 tolerance. For example, A=(selectivity 0.000, outcnt 3), B=(0.009, 2), and C=(0.018, 1) form a comparison cycle. This is not a strict weak ordering and cannot be passed to slices.SortFunc. Please define a transitive ordering key first, or leave both join-order sites on sort.Slice and track the underlying comparator separately.

3. parameterIntervalsCoverSegment has another tolerance-based, non-transitive comparator. sameGeometryCoordinate treats starts within 1e-9 as equal, then compares their ends. Starts A=0.5, B=0.5000000009, C=0.5000000018 with descending ends form a comparison cycle. A complete interval cover can consequently be ordered with an apparent gap, producing incorrect ST_CONTAINS, ST_COVERS, ST_EQUALS, or ST_OVERLAPS results. Please sort by an exact total order on start/end and reserve the epsilon comparison for the subsequent gap/merge logic; add a near-epsilon regression test.

The CI suite is green, but it does not cover these comparator-contract and unsupported-bound cases. The previously reported stats, txn, zone-map, and nil-ordering comparator fixes look addressed.

[XuPeng-SH]

I think this still needs changes before merge.

The earlier join-order and vector-distance-bound issues look materially improved at this head, but one remaining correctness blocker stands out:

parameterIntervalsCoverSegment now uses slices.SortFunc, but the comparator is still not a strict weak ordering because it treats starts within 1e-9 as equal and then compares ends.

Current code in pkg/sql/plan/function/func_binary.go:

• if sameGeometryCoordinate(a.start, b.start) { return cmp.Compare(a.end, b.end) }
• else return cmp.Compare(a.start, b.start)

That relation is non-transitive. Example: starts A=0, B=0.9e-9, C=1.8e-9. Then AB and BC, but A is not equivalent to C. With ends A=0.9, B=0.8, C=0.7, the comparator gives A>B, B>C, and C>A — a cycle. slices.SortFunc explicitly requires a strict weak ordering, so behavior is undefined here.

This is not just theoretical. lineSegmentCoveredByLineCollection builds these intervals and immediately assumes the sorted first element has the minimum start. If the cyclic triple above is ordered with the 1.8e-9 interval first, parameterIntervalsCoverSegment returns false at intervals[0].start > 1e-9 even though the intervals together cover the full segment. That can change line-based geometry predicate results in the LINESTRING/MULTILINESTRING paths that call it.

Suggested fix: sort by an exact total order on raw (start, end) and keep the epsilon logic only in the later merge/gap checks. Please also add a near-epsilon regression that proves complete coverage still returns true across this boundary case.

[fengttt]

@XuPeng-SH thanks — fixed in 221ffe0.

You're right, this was the same non-transitive-comparator class as the earlier ones, and it's a real correctness bug in the LINESTRING/MULTILINESTRING coverage path, not just theoretical. parameterIntervalsCoverSegment now sorts by an exact total order on (start, end):

slices.SortFunc(intervals, func(a, b geometryParamInterval) int {
    if c := cmp.Compare(a.start, b.start); c != 0 {
        return c
    }
    return cmp.Compare(a.end, b.end)
})

The epsilon (sameGeometryCoordinate) is removed from the sort and left only in the gap/coverage loop, where it's applied pairwise and doesn't need transitivity — exactly as you suggested.

Added TestParameterIntervalsCoverSegmentNearEpsilonBoundary using your cyclic-triple boundary case (starts {0, 0.9e-9, 1.8e-9} inside the 1e-9 band, ends {0.9, 0.8, 0.7}) plus a [0.85, 1.0] interval so the set covers all of [0,1]. I confirmed the old comparator returns a wrong false for some input orders of that set; the new one reports full coverage true in every input order (and a genuine gap still returns false).

Verified: unit test + the full geo BVT suite pass, go build ./.../vet/gofmt clean.

[XuPeng-SH]

Re-reviewed the latest head (245eac) after the geometry fix and the clean merge from main. The previous blocking comments are resolved.

Correctness: the migrated comparators now satisfy strict weak ordering, with stable sorting where equal-key order is semantically observable. The vector-distance planner folds repeated bounds to their true intersection, preserves unsupported bounds as residual predicates, and shares literal decoding with the reader. The geometry interval sort now uses an exact total order and applies epsilon only in the coverage logic. Join-stat ordering is transitive.

Performance: the conversions retain in-place O(n log n) behavior while removing interface/reflection boxing; no new unbounded work or hot-path allocation was introduced. The full basic benchmark passed on the geometry-fix parent commit.

Unhappy paths: checked nil/uninitialized zone maps and checkpoints, unsupported/non-literal and one-sided vector bounds, equal keys, input permutations, and Q1-Q3 resource/wait/growth risks. No new resource ownership, waits, goroutines, or unbounded accumulation were introduced.

Validation: all Go packages touched by the PR pass locally; the focused planner and geometry regressions pass 20 consecutive runs. The complete CI suite (UT, SCA, three BVT variants, coverage, and basic benchmark) passed on parent 221ffe0. The current head is a conflict-free main merge, its rerun has no failures so far, and merge protection will still wait for it.

[Copilot]

Pull request overview

Copilot reviewed 74 out of 75 changed files in this pull request and generated 1 comment.