Skip to content

Scan encoded expressions without traversal stacks - #138

Open
MesTTo wants to merge 1 commit into
trueagi-io:mainfrom
MesTTo:pr/expr-scan-no-stack
Open

Scan encoded expressions without traversal stacks#138
MesTTo wants to merge 1 commit into
trueagi-io:mainfrom
MesTTo:pr/expr-scan-no-stack

Conversation

@MesTTo

@MesTTo MesTTo commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

yeah, i know it's a constant-factor win, and the kernel's gated against those until the asymptotics have no known deficiencies (#126/#127). but this isn't a kernel-algorithm change, it's the expr codec's inner scan, it's byte-identical with a differential test, two files, ~40 lines of actual logic, and it's up to 29% fewer retired instructions across the default suite for a mechanical stack-to-counter swap. felt worth putting up.

item_source and ExprEnv::args walked an encoded expression with an explicit traversal stack — a SmallVec in item_source, a per-subterm traverseh! in args — pushing pending arities and popping on descent. The encoding is prefix order and every tag carries how many subterms follow, so one integer is enough: Arity(a) does pending += a - 1, each leaf (SymbolSize, NewVar, VarRef) decrements, and the walk ends when pending reaches zero. No stack, no per-item bookkeeping. The item_source hunk is +17/-26, it shrinks the function.

item_source is the byte-machine's inner scan, so dropping its stack shows up across the whole suite. Measured on mork bench, release target-cpu=native, min-of-3 perf stat -e instructions:u, against the parent commit. Outputs and internal step/unification counters are identical everywhere:

bench instructions:u
counter_machine −28.6%
process_calculus −18.8%
bfc −13.1%
exponential_fringe −12.0%
transitive −5.8%
odd_even_sort −2.8%
clique −1.1%

Byte-identical by construction (the item stream is unchanged), and pinned: generated_expression_scanners_match_recursive_reference runs 4096 random terms through the pending-count scanners and a recursive reference, comparing item for item and arg span for arg span. cargo test -p mork-expr is green (18 tests), mork test exits 0, and mork bench process_calculus reports the same unifications 201401, instructions 427949817 as the parent. Default build, no feature flags, no interface change.

`item_source` and `ExprEnv::args` walked an encoded expression by pushing
each pending arity onto a stack (a `SmallVec` in item_source, a per-subterm
`traverseh!` in args) and popping as they descended. The encoding is prefix
order and every tag says how many subterms follow, so one `pending` counter
suffices: `Arity(a)` sets `pending += a - 1`, every leaf (Symbol, NewVar,
VarRef) decrements it, and the walk ends when `pending` reaches zero. No
stack, no per-item bookkeeping allocation.

The item stream is unchanged, so this is byte-identical. A generated
differential (`generated_expression_scanners_match_recursive_reference`)
checks the pending-count scanners against a recursive reference over 4096
random terms, item for item and arg span for arg span.

item_source is the byte-machine's inner scan, so dropping its stack shows up
across the suite. Measured on `mork bench`, min-of-3 perf instructions:u,
outputs and step counters identical to the parent:

  counter_machine    -28.6%
  process_calculus   -18.8%
  bfc                -13.1%
  exponential_fringe -12.0%
  transitive          -5.8%
  odd_even_sort       -2.8%
  clique              -1.1%
@Adam-Vandervorst

Copy link
Copy Markdown
Collaborator

This was on my to list for a long time, but due to the "non-constant factor only" optimization policy I never got to it. The initial plan was to have an expression zipper as well, hence the stack, but let's simplify the traversal!

@Adam-Vandervorst

Copy link
Copy Markdown
Collaborator

I guess we can't replace it in traverseh because functions take the item index as an argument (which we're eliding here)?

@Adam-Vandervorst

Copy link
Copy Markdown
Collaborator

args should not be used in the places that show up in the benchmarks in the first place: the traversal if the expression is quadratic. coreferential transition for example should (probably) not re derive children during descend.

@MesTTo

MesTTo commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

on traverseh: you are right that i cannot replace it, but not for that reason. the index the
callbacks get is a byte offset from the expression's base, and the counter scan already carries
exactly that: the dispatch in args reads byte_item(*env.base.ptr.add(env.offset + se_offset)),
and se_offset there is the same j. i could hand it to a callback for free.

what blocks me is the payload. traverseh's stack is State { iter, payload }, and my counter
replaces iter only. it is a catamorphism, so there has to be one live accumulator per open
compound, and a single integer cannot hold those. the sharper problem i ran into is that a lone
counter cannot signal node close, so $finalize has nowhere to fire: [2][2] a b c and [3] a b c
both run the counter 1 to 0 across three leaves, and only a stack tells you where the inner node
ended. so i would leave traverseh as the general fold and not try to convert it.

i did go through all five remaining call sites, and they happen to be counter-compatible, because
they either have unit accumulators or a constant zero with an associative add and identity finalize.
but two are not reachable from the module tree, two are off the benchmark paths, and only
RelExprEnv::eq reads the offset at all, so i have no measured reason to touch any of them.

the stack i did miss, and which is worth the same treatment, is item_sink. it still has the
SmallVec this PR removes from item_source, and it is a pure streaming consumer with no payload
and no node-close callback, so the swap applies unchanged. i had it at 13.6% of bfc in my last
symbol profile. happy to add it here or send it separately.

on args and coreferential_transition: i agree, and i would put it more strongly than one call
being linear. one args is Θ(items in the subterm). calling it at every compound while descending
makes the descent Σ depth(item), so Θ(N·D̄): Θ(N log N) balanced, Θ(N²) on a chain, which is what a
copied Peano S^N in the exec stream is.

i am not deriving that from nothing, i have hit it once already. on the compile path
compile_match_factor called args at every Arity node to find child spans, i profiled
counter_machine at ~51% self in ExprEnv::args under that recursion, and replacing it with a
single preorder flatterm pass with a back-patch stack took it 1764us to 846us, byte-checked against
the retained recursive form. that rewrite is on another branch and is not in this PR.

where i want to be careful is the interpreter path you are pointing at, because i have already tried
the two obvious versions there and measured both neutral:

  • a pattern-arg memo keyed on the Arity arm: byte-identical, no speedup, and when i instrumented it i
    got a 98.7% miss rate, 2002 builds against 168,804 hits and 12,898,500 misses.
  • a per-match arg table the Arity arm pushes from instead of calling args: neutral, so i reverted it.

both failed for the same reason, and that is the part i think is actually useful here: almost all of
the Arity arm's args traffic is on transient VarRef-bound data subterms, not on the invariant
pattern. the VarRef arm pushes the matched data subterm onto the stack and the Arity arm then walks
it. so there is nothing to precompute and caching cannot help. what i take from that is that the fix
has to make the decomposition itself O(1) rather than memoize it.

which is where i think this PR's observation leads. the encoding is preorder-contiguous, so the Arity
arm does not need to explode into k children at all. a cursor { offset, v, pending } pops one
item, advances by that item's own width, does pending += a on Arity(a) and v += 1 on NewVar.
the v accounting that args currently recomputes per child becomes one increment at the moment the
variable is visited. that would make the arm O(1) and one push instead of arity, on pattern and
data alike, which is the specific reason i do not think it is the same experiment as the memo.

match_any_term in the same file is already exactly this shape, including the same pending - 1 + a
recurrence, and is already differential-checked against the interpreter. two caveats i should be
honest about: the stack is heterogeneous, with the synthetic NewVar envs and the bound-substitution
env, so the entry becomes an enum rather than a drop-in swap. and i have not built or measured this
one, so i am not claiming a number for it. happy to take it as the next increment if you want it.

one thing i should correct rather than let stand: the ~11.8% figure i have quoted for args before
comes from a profile predating this PR. i have not re-profiled symbols since, so i would not assert
a current share for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants