Skip to content

perf: fast path string self concat#138

Merged
joshcramer merged 2 commits into
mainfrom
perf/string-self-concat-fastpath
Jun 25, 2026
Merged

perf: fast path string self concat#138
joshcramer merged 2 commits into
mainfrom
perf/string-self-concat-fastpath

Conversation

@larimonious

Copy link
Copy Markdown
Contributor

Summary

  • Adds a narrow statement fast path for repeated string self-concat (s = s + piece) to avoid the O(n²) clone cliff.
  • Preserves fallback semantics for assignment expressions, call/side-effect RHS expressions, RHS errors, nested scopes, and TypeMode strict/warn/forgiving conversion behavior.
  • Adds examples/perf/string_concat_cli.tnt and updates DD-061 with measured before/after results and next-step guidance.

Benchmark

Local dev-release microbenchmarks against origin/main (0491a74):

  • s = s + "x", 100k iterations: median 0.770s0.030s
  • s = s + "x", 200k iterations: median 4.760s0.060s
  • s = s + piece + "y", 100k iterations: median 5.370s0.040s
  • s = s + piece + "y", 200k iterations: median 26.100s0.070s

Verification

  • cargo fmt
  • cargo build --profile dev-release
  • cargo test string_self_concat --lib -q
  • cargo test environment_append_to_string_binding --lib -q
  • cargo test array_self_append --lib -q
  • cargo test --lib -q
  • cargo test --test language_features_tests --test type_checker_tests --test cli_tests -q
  • cargo test
  • ./target/dev-release/ntnt docs --generate
  • ./target/dev-release/ntnt lint examples/perf/string_concat_cli.tnt
  • cargo fmt -- --check
  • git diff --check

@greptile-apps

greptile-apps Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR implements a narrow statement fast path for repeated string self-concat (s = s + piece) that avoids the O(n²) clone cliff by mutating the binding in place with push_str instead of allocating a new String on every iteration. The optimization is gated by a structural AST check, a side-effect guard on all collected RHS terms, and a runtime type check, so it falls back to the standard eval path for assignment expressions, calls, nested scopes, and any pattern it cannot prove safe.

  • src/interpreter.rs: adds binding_is_string, append_to_string_binding, try_eval_string_self_concat_statement, collect_string_self_concat_terms, and coerce_string_concat_rhs, plus a comprehensive test suite covering TypeMode strict/warn/forgiving, RHS errors, side-effect preservation, and alias safety.
  • examples/perf/string_concat_cli.tnt: new CLI microbenchmark fixture used to measure the before/after improvement.
  • design-docs/dd-061-interpreter-performance-roadmap.md: marks PR 7 complete, records measured benchmark deltas, and redirects to PR 8.

Confidence Score: 5/5

The change is safe to merge; the fast path activates only when a strict set of structural and runtime guards all pass, and the fallback to normal eval is exercised for every case that cannot be proven safe.

All three gate conditions (AST shape, side-effect freedom, binding type) are checked before any mutation occurs. coerce_string_concat_rhs faithfully mirrors the (String, b) arm of the existing binary + operator, preserving TypeMode semantics exactly. The test suite covers the aliasing edge case, RHS error atomicity, all three TypeMode tiers, function call fall-through, and the chained multi-term case.

No files require special attention.

Important Files Changed

Filename Overview
src/interpreter.rs Core fast-path implementation; logic is correct, TypeMode parity with normal eval verified, side-effect guard and binding type check properly ordered, tests thorough.
examples/perf/string_concat_cli.tnt New benchmark fixture exercising the chained text + piece + "y" pattern; straightforward and consistent with DD-061 benchmark description.
design-docs/dd-061-interpreter-performance-roadmap.md Documentation update marking PR 7 complete with measured results and updating the recommendation; no code changes.

Reviews (2): Last reviewed commit: "fix: document string concat accumulator ..." | Re-trigger Greptile

Comment thread src/interpreter.rs
@joshcramer joshcramer merged commit fa17cc7 into main Jun 25, 2026
15 of 17 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