chore(compiler): speed up typecheck for deep else-if ladders (remove quadratic complexity)#1865
Conversation
Locals assigned in if and else-if predicates remain visible after the chain; locks flatten semantics vs nested else-scope strip. Co-authored-by: Cursor Grok 4.5 Signed-off-by: Klondike Dragon <klondikedragon@gmail.com>
Peel parser else { if ... } into multi-arm IfStatement so
typecheck visits each arm once instead of re-walking nested
suffixes. Keeps predicate side-effect order and join semantics.
Co-authored-by: Cursor Grok 4.5
Signed-off-by: Klondike Dragon <klondikedragon@gmail.com>
Avoid deep-cloning exact object/array Collections and TypeDefs on every nested path write during progressive typecheck. Co-authored-by: Cursor Grok 4.5 Signed-off-by: Klondike Dragon <klondikedragon@gmail.com>
Wrap known fields in SharedMap so TypeState forks clone cheaply and writes copy-on-write through make_mut. Co-authored-by: Cursor Grok 4.5 Signed-off-by: Klondike Dragon <klondikedragon@gmail.com>
Wall-clock harness for synthetic ladders and --program files, plus --env-width/--env-depth/--env-seed-fields to seed a spine+stubs ExternalEnv Kind for more realistic compile timing. Co-authored-by: Cursor Grok 4.5 Signed-off-by: Klondike Dragon <klondikedragon@gmail.com>
TypeState forks (if arms, compile_expr snapshots) clone locals cheaply until make_mut; ~2.7x faster compile on a large --program workload in the ladder harness. Co-authored-by: Cursor Grok 4.5 Signed-off-by: Klondike Dragon <klondikedragon@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76838b148a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Signed-off-by: Klondike Dragon <klondikedragon@gmail.com>
Flattened typecheck treated later-predicate bindings as definite after the chain. Re-apply apply_child_scope from post-arm0 locals and expand if/else-if suite coverage. Co-authored-by: Cursor Grok 4.5 Signed-off-by: Klondike Dragon <klondikedragon@gmail.com>
Update: fixed so that the semantics are now exactly the same as beforeCodex flagged that flattening While that was an intentional choice in the original PR to make the behavior of else if predicates more like if predicates, because of how we were peeling, it would lead to an incorrect behavior for a block that did Fix: after merging arm states, re-apply Tests: expanded Running tests on main branch vrl: copied these new
So these goldens match pre-flatten nested semantics; this commit is not introducing new local-escape behavior and not changing semantics. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abe4e9fc7c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Signed-off-by: Klondike Dragon <klondikedragon@gmail.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Summary
Hi! I 💖 vector and have been using it for many years. thank you!
This PR speeds up VRL compile / typecheck for programs with long nested
else ifchains by removing O(n^2) behavior for this case.How we discovered it: We're using VRL to classify events in complex ways, which means some of our VRL has long else-if chains (40-60+ arms in several places). Our test suite (which runs validate over each VRL file + exercises data flow) went from a few seconds total to minutes as we've developed more VRL files. We found vector taking a long time (8+ seconds) to validate/compile some of our VRL files. Profiling pointed at
Expression::apply_type_info/IfStatement/Kind/TypeState/LocalEnvclone and merge.After these changes, these same VRL files validate/compile an order of magnitude faster (~8s to ~0.8s), taking our overall test suite run back from minutes to seconds.
While VRL compile-time is a one-time cost at startup, it's single-threaded and with our VRL, machines were taking 30+ seconds from vector startup before processing events. This removes that long delay.
I've contributed a smaller PR in the past, but this is my first time digging in to the compiler in VRL... I'm happy to adjust and discuss!
Change Type
Is this a breaking change?
Semantics note
After some updates to the PR, there are no changes to semantics. Additional tests were added to increase coverage of semantics for edge cases around if-else-if chains and related.
Problem
else ifto nestedelse { if ... }.IfStatement::type_infoclonesTypeState, typechecks both sides, then merges.Kindtrees and local bindings, so cost grows O(n^2) rather than a single linear pass over arms (O(n)).Approach
Ordered commits:
else { if ... }into a multi-armIfStatementat compile so each arm is typechecked once. Main benefit that solves quadratic behavior.Kindinserts and assignment type updates (avoid deep-clone on every nested write).Arc+ COW forCollectionknown maps.examples/else_if_ladder_compile.rs) with ladder +--programand--env-width/--env-depthknobs.Arc+ COW forLocalEnvbindings (this commit further speeds up ~2.7× on a large--programworkload in that example driver, on top of the earlier commits).apply_child_scopefrom the post-arm-0 local env so bindings first introduced in later predicates are not definite after the chain (restoring nestedelse { if }Block semantics).Benchmarks
Timings below are from my local release builds of
examples/else_if_ladder_compileSynthetic benchmark
Performance after primary fix of removing quadratic complexity:
After doing in-place mutations of
Kindplus cheap clones ofCollectionandLocalEnv(viaArc), the 60 arm test drops to med_ms=49.4:So approximately a 40x speedup for the 60-arm case before to final after. The performance is also the same after the 8th commit.
Synthentic benchmarks are interesting, but how about a real-world case...
End-to-end Vector validate (
bench-vrl-runtime.py)Our CI harness in one of its steps uses vector validate over all of VRL scripts. We exposed this piece of it to benchmark this piece (with our large VRL it was causing big slowdowns). We built vector 0.57 with our branch cherry-picked to the version of VRL paired with 0.57, and compared it to stock vector on my Linux system (was version 0.56, sorry it's not quite the same, I could re-run if you want; other systems where I'm running 0.57 show almost identical behavior though, and the profiling data showed the hotspot was the type checking and VRL synthetic benchmarks confirm that):
Our bench-vrl-runtime.py isn't part of the PR, it's something we wrote to interface with our VRL CI.
Same harness flags; PATH selects the Vector binary.
Stock (
/usr/bin/vector0.56.0 release on this machine):PATH=/usr/bin:$PATH python3 tools/bench-vrl-runtime.py \ --scenario mix --events 1 --arms full --work /tmp/sl-vrl-runtime-benchPatched (workspace Vector 0.57.0 release build pinned to this VRL):
So
cfg-fullvalidate ~8.07s → ~0.79s on that A/B.How did you test this PR?
make allgreenexpressions/if_statementfiles: first-ifpredicate escape, an assert-heavy success file for cross-arm / type-merge / short-circuit cases, and E701 goldens for else-if pred escape, bracedelse { if }, else-if body escape, body-->later-pred isolation, multi-stmt else, and third-arm pred + finalelseelse_if_ladder_compileDoes this PR include user facing changes?
our guidelines.
Checklist
run
dd-rust-license-tool writeand commit the changes. More details here.AI disclosure
I used Cursor Grok 4.5 to help develop this PR (understanding the compiler code base, developing a strategy, writing code and tests). I've personally reviewed the code and believe it's correct. This is my first time in the VRL compiler codebase though, so I greatly appreciate your expertise in reviewing!
References
If you like this PR, consider reviewing my unrelated vector PRs I'm writing up for vectordotdev/vector#25810 and vectordotdev/vector#25815