Repository navigation
Eliminate quadratic environment rebuilds in Scope::get - #439
Merged
stringhandler merged 2 commits intoOct 13, 2026
Merged
Conversation
Every variable read in codegen re-folded all live bindings into the input pattern, re-converted it, and re-walked it to find the variable: O(n) work per read and O(n^2) per program, even though the generated program is only linear in size. A 4096-binding program spent ~3.9s in codegen. Scope now maintains its bindings incrementally: a flat list of binding patterns, an identifier -> binding-index map, and per-scope start offsets. Reading an identifier (get_identifier) computes its selector directly from the binding's position: drop past newer bindings, take into the target binding (skipped for the oldest binding, the right tip of the input spine), then select within that binding's own pattern via the new BasePattern::get_from. The selectors are identical to the old fold-and-translate result, which the tests keep as a reference oracle, so the compiled program is byte-for-byte unchanged. - pattern.rs: factor BasePattern::get into get_from, which continues an accumulated selection instead of starting from an empty one - compile/mod.rs: rework Scope's representation as above; add a randomized property test comparing get_identifier against the reference, covering main and child scopes, shadowing and compound patterns Validation: the workspace test suite passes, including the pinned test-data bytecode regressions, and 28 programs (all examples plus generated stress shapes) produce byte-identical output to master. Local benchmarking (release build): codegen -68% for 256 sequential bindings, -63% for 256 bindings across 32 nested scopes, -35% at 64 bindings; a 4096-binding program goes 3.90s -> 120ms. Programs with only a few bindings regress ~1-3% from the per-let bookkeeping. Assisted-by: Codex (rebase) Co-authored-by: Tommy Volk <tvolk131@gmail.com>
The three fields added by the previous commit (binding list, identifier -> binding-index map, per-scope start offsets) form a referential-integrity triple: the index fields are meaningless without the binding list, and pop_scope repairs all three together. No invariant crosses the other Scope fields. ScopeBindings owns the triple behind a small interface (from_root/insert/push_scope/pop_scope/position_of/as_slice), so the invariants hold at method boundaries and pop_scope's transient half-updated states stay unobservable. The type has no generic parameters; all selector and type-inference logic stays in Scope. A property test checks position_of against a naive newest-first rescan of the bindings after every mutation. Also drops a stale "## Panics" section from insert, a leftover from a body that could panic; the current one cannot. Pure refactor: tests pass and example and stress programs produce byte-identical output. Assisted-by: Codex (rebase) Co-authored-by: Tommy Volk <tvolk131@gmail.com>
KyrylR
approved these changes
Oct 13, 2026
KyrylR
left a comment
KyrylR
left a comment
Collaborator
There was a problem hiding this comment.
ACK f4ca6d6; successfully ran local tests
Idea and implementation are sound, though the code and test complexity worry me
I would prefer to have more "functional" approach for structuring the code, but imo the current solution does a job in fixing a bug, therefore it is good enough
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Redo of #409
Redid this PR with signed commits. Added tvolk131 as a co-author in commit messages.
What this fixes
Compiling a function with many
letbindings was quadratic in the number of bindings. Every variable read rebuilt the entire input environment from scratch: it re-folded all live bindings into a tree, converted that tree, and then searched it for the variable.What changed
Scopenow maintains its bindings incrementally: a flat list of binding patterns, a map from identifier to binding index, and per-scope start offsets. Reading a variable computes its take/drop selector directly from the binding's position: drop past the newer bindings, take into the target binding, then select within that binding's own pattern. The resulting selectors are identical to what the old code produced, so the compiled program is unchanged.Correctness
Performance
Local benchmarking on an Apple M4 Pro, release build:
Codegen speed is effectively completely linear with respect to the number of bindings now. A program with 4096 bindings (which is 64 * 64) takes 60x longer than one with 64 bindings.
Commits
Two commits. The first is the optimization. The second is a pure refactor that moves the three coupled binding fields into a
ScopeBindingstype with its own invariant test. Happy to drop the second if you prefer the fields onScope.The benchmark and profiling harness behind these numbers lives on my fork (bench/compiler-perf). Happy to send it as a separate PR if useful.