Skip to content

perf: fast path array self append#137

Merged
joshcramer merged 2 commits into
mainfrom
perf/array-self-append-fastpath
Jun 23, 2026
Merged

perf: fast path array self append#137
joshcramer merged 2 commits into
mainfrom
perf/array-self-append-fastpath

Conversation

@larimonious

Copy link
Copy Markdown
Contributor

Summary

  • Adds a narrow fast path for loop/control-flow statement contexts shaped like arr = arr + [item] / [items...].
  • Mutates the resolved array binding in place instead of cloning the growing array on every append.
  • Preserves generic + behavior for non-target shapes, including call-containing RHS arrays, assignment-expression results, RHS error behavior, and alias/copy semantics.
  • Updates DD-061 with PR 6 completion notes and measured benchmark results.

Benchmarks

  • Before: 20k append CLI fixture: 7.11s
  • After: 20k median: 0.0122s
  • After: 50k median: 0.0238s
  • After: 100k median: 0.0439s

Verification

  • cargo fmt -- --check
  • cargo build --profile dev-release
  • cargo test --lib -q — 1153 passed
  • cargo test --test language_features_tests --test type_checker_tests --test cli_tests -q
  • ./target/dev-release/ntnt docs --generate
  • git diff --check

@greptile-apps

greptile-apps Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR introduces a narrow in-place fast path for the arr = arr + [item] idiom inside loop bodies, eliminating the O(n²) full-array-clone cliff. It also fixes a latent bug in eval_block where statement errors skipped deferred statement execution and left the interpreter's scope pointing at the child environment — the fix is unified into the new eval_block_inner helper and applied to both the new control-flow path and the existing non-control path.

  • Fast path (try_eval_array_self_append_statement): activated only when the exact assignment shape matches, all RHS elements pass a conservative expression_is_side_effect_free check, and the binding is confirmed to be an array; mutates the stored binding in place, skipping the intermediate clone.
  • eval_block deferred-statement fix: eval_block_inner now captures errors, drains and executes deferred statements (LIFO), restores the previous environment, then re-propagates — matching the correct behaviour already implemented in the for-loop env-restore path.
  • DD-061 update: renumbers PRs 6–10 on the roadmap and records measured benchmark results (20k appends: 7.11 s → 12.2 ms).

Confidence Score: 5/5

Safe to merge. The fast path is narrow and well-guarded: it only fires when the exact assignment shape matches, all RHS elements pass an exhaustive side-effect-free check, and the binding is confirmed to be an array before any mutation. Errors during element evaluation abort before any in-place append.

The fast path guards are conservative and exhaustive — all 27 Expression variants are handled in expression_is_side_effect_free, the TOCTOU window between binding_is_array and append_to_array_binding is sealed by the side-effect-free predicate, and partial mutation on RHS error is prevented. The eval_block deferred-statement fix is backward-compatible and directly tested. The 1153-test suite, including new alias, error-abort, and deferred-drain tests, gives high confidence no regressions were introduced.

No files require special attention.

Important Files Changed

Filename Overview
src/interpreter.rs Core changes: new binding_is_array/append_to_array_binding Environment helpers, try_eval_array_self_append_statement fast path, exhaustive expression_is_side_effect_free predicate (all 27 Expression variants covered), unified eval_block_inner that correctly runs deferred statements on error in both control and non-control contexts, and eval_block_for_control wiring into while/loop/for statement handlers. Logic is sound; alias semantics, partial-append prevention, and scope restoration are all tested.
design-docs/dd-061-interpreter-performance-roadmap.md Documentation-only: PR 6 rewritten to describe the array self-append fast path, PR 7 added for string self-concat, existing PRs 6–8 renumbered to 8–10, priority table updated, benchmark deltas recorded. No code impact.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["eval_block_for_control(block)"] --> B["eval_block_inner(block, control=true)"]
    B --> C["for each stmt"]
    C --> D["eval_statement_for_control(stmt)"]
    D --> E{Statement kind}
    E -- "Located{stmt}" --> F["set line/col, recurse"]
    F --> D
    E -- "Expression(expr)" --> G["try_eval_array_self_append_statement(expr)"]
    G --> H{Shape match?}
    H -- "Not Assign / lhs≠rhs / no array binding or elements have side effects" --> I["eval_expression(expr) — generic path"]
    H -- "arr = arr + literal + binding_is_array passes" --> J["eval each element"]
    J --> K{Any element error?}
    K -- "Err" --> L["propagate error, no mutation"]
    K -- "Ok(items)" --> M["append_to_array_binding() — mutate in-place"]
    M --> N["return Ok(Value::Unit)"]
    E -- "other statement" --> O["eval_statement(stmt) + control_flow_or_unit"]
    B --> P{stmt error?}
    P -- "Err captured" --> Q["drain & run deferred stmts, restore env, propagate Err"]
    P -- "Break/Continue/Return" --> R["break loop, run deferred stmts, restore env, return control value"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["eval_block_for_control(block)"] --> B["eval_block_inner(block, control=true)"]
    B --> C["for each stmt"]
    C --> D["eval_statement_for_control(stmt)"]
    D --> E{Statement kind}
    E -- "Located{stmt}" --> F["set line/col, recurse"]
    F --> D
    E -- "Expression(expr)" --> G["try_eval_array_self_append_statement(expr)"]
    G --> H{Shape match?}
    H -- "Not Assign / lhs≠rhs / no array binding or elements have side effects" --> I["eval_expression(expr) — generic path"]
    H -- "arr = arr + literal + binding_is_array passes" --> J["eval each element"]
    J --> K{Any element error?}
    K -- "Err" --> L["propagate error, no mutation"]
    K -- "Ok(items)" --> M["append_to_array_binding() — mutate in-place"]
    M --> N["return Ok(Value::Unit)"]
    E -- "other statement" --> O["eval_statement(stmt) + control_flow_or_unit"]
    B --> P{stmt error?}
    P -- "Err captured" --> Q["drain & run deferred stmts, restore env, propagate Err"]
    P -- "Break/Continue/Return" --> R["break loop, run deferred stmts, restore env, return control value"]
Loading

Reviews (2): Last reviewed commit: "fix: restore block scope on errors" | Re-trigger Greptile

Comment thread src/interpreter.rs Outdated
@larimonious

Copy link
Copy Markdown
Contributor Author

Addressed Greptile's feedback in ebedea5:

  • unified normal/control block evaluation through a shared eval_block_inner path
  • eval_block now restores the previous environment and drains deferred statements before returning statement errors
  • added regression tests for scope restoration and deferred execution on block errors
  • made the array self-append fast path fail closed if its post-guard append invariant ever breaks, avoiding accidental generic fallthrough/double evaluation

Local verification passed: cargo fmt -- --check, cargo build --profile dev-release, targeted tests, full lib tests, integration tests, docs generation, and git diff --check.

@joshcramer joshcramer merged commit 0491a74 into main Jun 23, 2026
9 checks passed
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