Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 93 additions & 2 deletions docs/adrs/adr-0023.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,84 @@ and the rename only moves an entry that the floor has already made private to it
of one def's way. `ModuleRun::display_name` maps the internal name back wherever a def's
name reaches a message (the recursion-depth error), so it never surfaces.

*Points 2 and 3 are superseded by the #2955 amendment below: dependency bodies are no
longer wrapped into the defs that reach them, so there is no clash to rename. Point 1 stands.*

### Amendment (#2955, implemented): a dependency module is linked once, and forwarded to

Binding a module's dependencies by **copying** their bodies into every def that reached them
compounded down a chain: each level's bodies already carried the level below, so a chain
whose defs each call `F` defs of the level below held `F^L` copies of the bottom one. The
issue's 14-level, fan-out-2 chain peaked at 681 MB against jq's 2 MB (release, correct
output throughout). The "symbolic binding at load time" listed under the alternatives below
is now taken, in a form that changes neither the marker representation nor the evaluator:

1. **One link run per dependency module.** A module some other module depends on is emitted
**once**, as an ordinary run (`wrap_run`) placed outermost in `process_program` -- outside
the imports, `~/.jq` and includes -- whose begin marker carries a hidden alias
(`ModuleRun::link_alias`, `<NUL>link:<id>`) and whose defs are named
`ModuleRun::link_name` (`<NUL>link:<id>::<name>`). Inside the run a bare sibling call
misses those names, floors at the run's own marker, and is retried under the alias by the
very code decision 3 added for `import`ed modules; the retry renames the call in place, so
the evaluator binds it unchanged. From outside, nothing lexable spells a link name, so the
run re-exports nothing. Error attribution reads the innermost open run as before, so a body
error names the dependency's own file -- and, since the body now exists once, it is
reported once (#3058, closed by this).
2. **Forwarding stubs in place of copies.** `load_and_bind_module` wraps each of a module's
own def bodies in one stub per dependency (name, arity) the body calls directly --
`def g(a; b): <NUL>link:<id>::g(a; b);` -- with the two exclusions the copying loader had
(the def's own (name, arity), and a parameter's name at arity 0). A stub holds no lexable
free name, no `$variable` and no `$__loc__`, so it needs no run and can capture nothing;
the transitive closure and the rename above are therefore gone. Arguments forward as
closures, and the dependency does its own `$param` binding on arrival.
3. **The one resolver change.** `scan_scope` steps over an open begin marker -- instead of
stopping at it as the floor -- exactly when the name being looked up is a link name. A stub
sits inside some run of its own and its target is outside every run. The two lookups that
can meet a link name (`in_scope`, `reach_in_scope`) compute the flag from the name; variable,
label and `enclosing_run` scans never cross.
4. **Placement decides compile-error order.** Link runs are wrapped in dependency post-order
with each module's directives visited last-declared first (`ModuleLoader::hoist_order`), so
`resolve::check` meets a dependency's bodies before its includer's, the last-declared
dependency's first, and a chain's deepest first -- which is jq 1.7.1's order (captured live
for all three shapes; the copying loader printed the includer's own errors first).
5. **Only what is referenced is linked.** jq's `block_bind_referenced` rule at the module
level: a linked module emits a def only if a body reached by name from the main filter --
through the top-level runs, then the linked modules dependents-first -- names it. Every
chain def is installed over the whole program below it at evaluation, so chain length is
the cost that matters; seeding from every top-level def instead cost 22 MB against 12 MB on
a six-level, forty-def chain the filter walks one def of.

**Soundness, re-derived from decision 5.** A link run contains no bare-named def, so a bare
name the resolver accepts anywhere resolved to a def inner to every link run, which the
evaluator's innermost-first install also picks first. A link name exists only in its own run,
and same-name entries there are innermost-first in both passes. There is no input on which
the two pick different defs. The 30 rows of `test_dependencies_are_bound_in_their_own_scope_2962`
are unchanged.

**Measured** (this machine, release, `/usr/bin/time -l`, output identical to jq on every row):
the 8 x 6 x 3 chain 142 MB -> 34 MB, 12 x 4 x 2 162 MB -> 36 MB, 14 x 4 x 2 681 MB -> 112 MB
and 0.38 s -> 0.03 s; the flat 6 x 40 x 1 chain 12 MB -> 9 MB, and a 1000-def utility module
used for one function through two modules 9 MB -> 9 MB. The processed program's node count is now linear in chain depth
(`link_size_guard_2955`, which read 1253 / 5093 / 20453 nodes at 6 / 8 / 10 levels before).
What remains above jq's 2 MB is the evaluator's own: a `DefCall` node caches its bound body,
and a call tree of `F^L` calls leaves `F^L` cached copies behind, module or no module -- the
same 56 defs in one file cost 58 MB, and `fib(20)` 234 MB. That is a separate, pre-existing
evaluator issue, #3148.

**Hot path** (Apple M4 Pro, idle, interleaved, 25 reps, median): `[range(1e5) | f]` where `f`
calls one dependency through a stub moved +1.6% while the same defs written inline in one
program, which this change cannot touch, moved +3.3% -- neutral within the noise floor.

**The one shape that pays** (same box and method): a 20-module x 50-def chain of which the
filter uses one def starts in 9 ms against 20 ms before; the same chain with all 50 top-level
defs used, so that all 950 dependency defs are referenced and linked, starts in 136 ms against
32 ms. Every def in the top-level chain is installed over the
whole program below it when it is bound (`bind_def` rebuilds its `then`), so a chain of `M`
defs costs `O(M x N)` at startup; the copying loader kept those 950 bodies nested inside the
defs that used them, where each install was local. That quadratic is the evaluator's, and
pre-existing -- a single top-level `include` of 3000 defs costs 1 GB and 0.5 s today -- and it
is what the referenced closure above bounds by usage. It is filed with the retention issue, #3148.

## Alternatives rejected

- **A field on `Expr::FuncDef`.** Adding any field costs 8 bytes on *every* `Expr` — the
Expand All @@ -146,7 +224,15 @@ name reaches a message (the recursion-depth error), so it never surfaces.
than inlining an AST). This is the eventual answer to #2955's memory compounding and
would make the boundary structural rather than marked. It is a far larger change to the
loader and evaluator both, and is deliberately left as future work: nothing in this ADR
blocks it, and the markers disappear with the chain if it ever lands.
blocks it, and the markers disappear with the chain if it ever lands. *Taken by the #2955
amendment above, in a form that keeps the markers and leaves the evaluator untouched: the
handle is a hidden name, and the binding is the alias retry the resolver already had.*
- **Splicing dependency bodies behind `Expr::Shared`** (#2955's own first suggestion).
Unsound today on two counts: `resolve::check` un-shares a multi-owner `Rc` through
`Rc::make_mut` (#2971), so the memory comes straight back at compile time, and a shared
body's free names would resolve against whichever scope each splice site sits in, which is
#2962's capture family again. Linking once and forwarding shares nothing and resolves each
body in exactly one scope.

## Consequences

Expand Down Expand Up @@ -180,6 +266,11 @@ name reaches a message (the recursion-depth error), so it never surfaces.
- [#2962](https://github.com/rust-works/succinctly/issues/2962) — dependency capture;
closed by the amendment above
- [#2950](https://github.com/rust-works/succinctly/issues/2950) — shadow-candidate seeding
- [#2955](https://github.com/rust-works/succinctly/issues/2955) — memory compounding
- [#2955](https://github.com/rust-works/succinctly/issues/2955) — memory compounding;
closed by the amendment above
- [#3058](https://github.com/rust-works/succinctly/issues/3058) — a dependency's compile
error reported once per copy; closed by the same amendment
- [#3148](https://github.com/rust-works/succinctly/issues/3148) — the evaluator's per-call
bound-body retention and chain install cost, which is what remains
- [ADR-0018](adr-0018.md) — reference fidelity; this is conformance to its per-mode rule,
not an amendment to it
Loading
Loading