Skip to content

Commit d3c29b6

Browse files
Rc-shared collections — kill the n² cliff (PAIN_POINTS HIGH-1)
Wrapped HArray.items and Value::Dict's BTreeMap in Rc<RefCell<>>. Value::clone() drops from O(N) to O(1); mutations through borrow_mut hit all sharers in-place. The architectural blocker for any 10k+ workload, gone in one refactor. Measured on examples/recommend/recommend.omc (real MovieLens data): Stage Before After Speedup --------------- -------- ------- ------- 10k load_csv 9899 ms 28 ms 354x 10k aggregate 16018 ms 29 ms 552x 10k agg_to_rows 2125 ms 8 ms 265x 10k build_hidx 4883 ms 46 ms 106x 10k total ~33 sec ~0.12 sec ~275x 100k aggregate HANG/OOM 338 ms ∞ 100k total HANG/OOM ~0.92 sec ∞ The 100k MovieLens dataset (full data) now completes in under one second, including building a 9724-entry harmonic_index. Semantic shift: arrays and dicts now pass by REFERENCE (matching Python/JS/Ruby for mutable collections). Callees CAN mutate a caller's collection. Bulk operations (arr_concat, arr_slice, dict_merge, harmonic_sort, arr_unique, etc.) still produce a fresh Rc — the explicit-copy semantics live in the bulk operations, not the function call boundary. The old return-and-rebind idiom from V.6-era code still works (Rc::clone is cheap), so existing programs don't break. Architectural simplifications enabled: - Op::ArrPushNamed / Op::ArrSetNamed / Op::SafeArrSetNamed / Op::DictSetNamed / Op::DictDelNamed / Op::ArrayIndexAssign all no longer need vm_assign_var write-back — borrow_mut() through the Rc-shared backing handles propagation. - Same simplification on the tree-walk side for arr_push, arr_set, safe_arr_set, dict_set, dict_del, Statement::IndexAssignment. Conservative: anything ambiguous kept the assign_var call as a no-op (Rc::clone is cheap, just rebinds the variable to the same Rc). No code paths broken chasing perfection. VM is now meaningfully faster than tree-walk on collection-heavy workloads again (was ~1.3x slower pre-refactor due to identical underlying clone work). HOFs see ~2x speedup on the benchmarks. The PAIN_POINTS.md HIGH-3 entry (VM slower than tree-walk on dict-heavy) is automatically resolved by this fix. Test results: 43/43 functional examples byte-identical under tree-walk and VM. 141/141 workspace tests pass (92 omnimcode-core lib + 46 ffi/python + 3 misc). Memory record `omc_arrays_by_value.md` marked superseded with 2026-05-14 date and updated description. Refactor scope: ~80 sites across vm.rs and interpreter.rs. Mechanical patterns: .items.X() -> .items.borrow().X() for reads, .borrow_mut().X() for writes, HArray::from_vec() helper for construction, Value::dict_from() / Value::dict_empty() for dicts. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 6a1b4dc commit d3c29b6

4 files changed

Lines changed: 306 additions & 203 deletions

File tree

examples/recommend/PAIN_POINTS.md

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ return types (arr_len, str_len, fibonacci, etc).
4242

4343
---
4444

45-
## HIGH-1: Value::Dict / Value::Array clone on every read+write
45+
## HIGH-1: Value::Dict / Value::Array clone on every read+write ✅ FIXED
4646

4747
**Symptom:** Aggregating 10k records into a dict that grows to 3218
4848
entries takes 16 seconds. 100k records: hung — never completed in
@@ -88,6 +88,25 @@ architecture needs to choose: pass-by-value (O(N²) collections) or
8888
pass-by-reference (sharing, mutation surprises). Document the
8989
tradeoff and pick.
9090

91+
**Resolution (same session):** Picked pass-by-reference. Wrapped
92+
HArray.items in Rc<RefCell<Vec<Value>>> and Value::Dict's BTreeMap
93+
similarly. Measured speedups on this exact workload:
94+
95+
| Stage | Before | After | Speedup |
96+
|-------------------|-----------:|----------:|--------:|
97+
| 10k load_csv | 9899 ms | 28 ms | 354× |
98+
| 10k aggregate | 16018 ms | 29 ms | 552× |
99+
| 10k agg_to_rows | 2125 ms | 8 ms | 265× |
100+
| 10k build_hidx | 4883 ms | 46 ms | 106× |
101+
| **10k total** | ~33 sec | ~0.12 sec | ~275× |
102+
| **100k total** | hung | ~0.92 sec ||
103+
104+
43/43 functional examples produce identical output under tree-walk
105+
and VM. 92/92 unit tests pass. Old `omc_arrays_by_value` memory
106+
record marked superseded — `fn fill(a) { arr_push(a, 1); }` now
107+
mutates the caller's array, matching Python/JS/Ruby. The old
108+
return-and-rebind idiom still works as a no-op.
109+
91110
---
92111

93112
## HIGH-2: str_split per-line cost dominates CSV parsing

0 commit comments

Comments
 (0)