Skip to content

Commit 952c0be

Browse files
committed
fix(jsoo): make List.remove_assoc tail-recursive via the List shadow
add-all at size 10000 overflowed the default Node stack under js_of_ocaml: ensure_current_tx_tempid walks the tempid order list (one entry per transacted entity) with the non-tail stdlib remove_assoc, producing ~10k nested frames plus caml_compare calls. Extend the existing Datascript_types.List shadow (already used for concat_map/concat/flatten) with a tail-recursive remove_assoc so native, Melange, and jsoo all get bounded-stack behavior. The full size-10000 bench suite now completes under node --stack-size=200.
1 parent fabdb72 commit 952c0be

2 files changed

Lines changed: 31 additions & 8 deletions

File tree

‎docs/perf.md‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,21 @@ Current status:
110110
- `js_of_ocaml` is faster than upstream on nearly all cases; exceptions are
111111
`add-all` at size 10000 (2105 ms vs 941.57 ms) and `q3` at sizes 1000 and
112112
10000 (1.61 ms vs 1.31 ms at 10000).
113-
- `js_of_ocaml` overflows the default Node.js stack at size 10000
114-
(`RangeError: Maximum call stack size exceeded` during `add-all`). The
115-
numbers above were measured with `node --stack-size=8000`; this is a known
116-
js_of_ocaml recursion limitation, not a benchmark artifact.
113+
- `js_of_ocaml` used to overflow the default Node.js stack at size 10000
114+
(`RangeError: Maximum call stack size exceeded` during `add-all`), so the
115+
size-10000 numbers above were measured with `node --stack-size=8000`. The
116+
recursion was `List.remove_assoc` — non-tail in the OCaml stdlib — walking
117+
the per-transaction tempid order list in `ensure_current_tx_tempid`
118+
(`impl/transact.ml`), one entry per transacted entity (~10000 frames plus
119+
`caml_compare` calls). A tail-recursive `remove_assoc` now ships in the
120+
`Datascript_types.List` shadow (`type/datascript_types.ml`), next to the
121+
existing `concat_map`/`concat`/`flatten` overrides, so native, Melange,
122+
and js_of_ocaml all get the bounded-stack version. Verified: the full
123+
size-10000 suite now completes under `node --stack-size=200`, and
124+
`add-all` at size 10000 measured 764–923 ms (jsoo) vs 245 ms (native) on
125+
the same machine (before the fix: 825 ms jsoo / 245 ms native, measured
126+
with the default Node stack which happens to fit the ~10k-deep recursion
127+
on Node 24).
117128
- `storage-roundtrip` has no upstream equivalent (upstream bundle is in-memory
118129
only), so it is reported without a comparison.
119130
- `get-page-data` models Logseq's `logseq.api.db-based.tools/get-page-data`:

‎type/datascript_types.ml‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -534,10 +534,14 @@ type tx_report =
534534
(* The Melange stdlib compiles List.concat_map/concat/flatten into recursive
535535
JS calls whose depth grows with the input (concat_map once per element
536536
mapped to [], concat/flatten once per sublist), which overflows the JS
537-
call stack on data-sized lists such as query binding sets. These
538-
tail-recursive versions keep the exact Stdlib.List semantics on every
539-
platform. Declared here so the universal `open Datascript_types` (and
540-
`open Datascript`) shadows them module-wide. *)
537+
call stack on data-sized lists such as query binding sets. js_of_ocaml
538+
hits the same wall through the OCaml stdlib: List.remove_assoc is not
539+
tail-recursive there either, and walking a data-sized assoc list (e.g.
540+
the tempid order list, one entry per transacted entity) overflows the JS
541+
call stack where native barely notices. These tail-recursive versions
542+
keep the exact Stdlib.List semantics on every platform. Declared here so
543+
the universal `open Datascript_types` (and `open Datascript`) shadows
544+
them module-wide. *)
541545
module List = struct
542546
include List
543547

@@ -550,4 +554,12 @@ module List = struct
550554

551555
let concat l = concat_map (fun x -> x) l
552556
let flatten = concat
557+
558+
let remove_assoc x l =
559+
let rec aux rev_prefix = function
560+
| [] -> List.rev rev_prefix
561+
| ((a, _) as pair) :: rest ->
562+
if a = x then List.rev_append rev_prefix rest else aux (pair :: rev_prefix) rest
563+
in
564+
aux [] l
553565
end

0 commit comments

Comments
 (0)