perf(sorted_set): compute union size from split drops instead of recounting

[bobzhang]

Partially addresses #3824 — resolves the TODO: optimize this. Avoid counting the size of the set in union.

Before

After building the merged tree, union recomputed the result size with a full each() traversal (an extra O(n+m) pass over the freshly built tree).

After

An element common to both sets is dropped exactly once, at the found pivot of the split. union counts those drops and derives the size arithmetically: |self| + |src| − dups. The recount pass is gone.

Rebased reimplementation (2026-08-20)

When this PR was first written, union's split was the only splitter, and it gained an Int drop-count component. main has since grown split_member (the split-based difference/intersection/symmetric_difference rewrite), whose found flag carries exactly the same information — so the rebase re-implements the intent on today's machinery:

• union uses split_member and counts found pivots.
• The now-unused split is deleted; its whitebox test is replaced by an equivalent split_member test pinning found for present/absent/below-range/above-range/empty splits.
• split_member returns a #valtype struct (SplitResult { left; found; right }) instead of a 3-tuple, keeping the per-node result stack-allocated on native — same pattern as JsonNumberScan in json/lex_number.mbt. This benefits every split-based operation, not just union.

Benchmarks (moon bench --release, n=10K sets)

union, main → this branch (recount removal + valtype):

case	native	js	wasm-gc
50% overlap	518 → 429 µs (−17%)	297 → 222 µs (−25%)	197 → 158 µs (−20%)
disjoint	382 → 304 µs (−20%)	261 → 153 µs (−41%)	154 → 96 µs (−38%)
identical	592 → 520 µs (−12%)	323 → 277 µs (−14%)	232 → 193 µs (−17%)

#valtype struct vs tuple, isolated (affects union/difference/intersection/symmetric_difference): native −5% to −9%, js flat, wasm-gc within ±2% noise.

Tests

Union exact-size assertions for overlapping/disjoint/identical operands cross-checked against an element count; the split_member whitebox test; the existing invariant and quickcheck suites all exercise the new path. Mutation-verified: double-counting dups fails 8 tests.

Review

Reviewed by Codex CLI (xhigh) on the rebased reimplementation, first-round sign-off: the dups induction re-verified on split_member (disjoint fragments, pivot counted exactly once, empty-side arms cannot hide common elements), sizes read from unmutated copies, SplitResult preserves tuple semantics at all four call sites, no split callers remain, generated interface byte-identical. (The original pre-rebase version carried its own sign-off with the equivalent induction.)

Signed-off-by: Codex CLI codex@openai.com

Validation

• Full repo moon test: 7463/7463 (sorted_set 71/71)
• moon check --warn-list +unnecessary_annotation clean, moon fmt applied, no .mbti changes

🤖 Generated with Claude Code

[coveralls]

Coverage Report for CI Build 6217

Coverage decreased (-0.006%) to 90.718%

Details

• Coverage decreased (-0.006%) from the base build.
• Patch coverage: 13 of 13 lines across 1 file are fully covered (100%).
• No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	17937
Covered Lines:	16272
Line Coverage:	90.72%
Coverage Strength:	332417.78 hits per line

---

💛 - Coveralls

[bobzhang]

Codex CLI review — post-rebase (xhigh)

The branch was reimplemented on today's main (the original targeted a base without split_member; the rebase moves union onto it, deletes the dead split, and adds the #valtype SplitResult struct). Codex CLI reviewed the reimplementation fresh, briefed to re-verify the duplicate-count induction on the new recursion shape, the size-at-entry arithmetic, left/right mapping at all four SplitResult call sites, split's deletion, and the benchmark claims.

Verdict — approved, first round

No blocking findings.

The duplicate-count induction holds: split_member partitions b into disjoint < va and > va fragments, while found accounts for the pivot exactly once; empty-side arms cannot contain common elements. Both inputs are copied before the merge and their stored sizes remain unchanged.

SplitResult preserves the tuple semantics, and all callers retain the correct left/right mapping. No old split callers remain, and the generated interface is byte-identical.

Dropping the 101-element splitter case is not blocking: the deeper randomized invariant scenarios already validate AVL structure, node count, isolation, and subsequent mutation.

The commit-message benchmark percentages match the supplied measurements.

Signed-off-by: Codex CLI codex@openai.com

---

Posted on behalf of Codex CLI by Claude Code. Validation outside Codex's read-only sandbox: full repo moon test 7463/7463; sorted_set 71/71; benchmarks on native/js/wasm-gc as tabled in the PR body; dups += 2 mutation fails 8 tests.

[Copilot]

Pull request overview

This PR optimizes @sorted_set’s SortedSet::union by removing the post-merge each() traversal used to recompute the result size, addressing the performance TODO noted in #3824. It derives the union size as |self| + |src| - dups, where dups is counted during split_member pivots, and refactors split_member to return a #valtype result struct (replacing the old tuple and deleting the now-unused split helper).

Changes:

• Compute union result size via counted duplicate pivots (found) instead of recounting via each().
• Replace tuple-returning split_member with a #valtype SplitResult struct and update all internal call sites; delete the unused split implementation.
• Add a regression test asserting union’s stored size matches actual element count for overlap/disjoint/identical cases.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
sorted_set/set.mbt	Removes union recount pass by counting split_member.found duplicates; refactors split result to a #valtype struct and deletes unused split.
sorted_set/set_test.mbt	Adds regression coverage ensuring union.length() remains exact and matches a real traversal count.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.