Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions design-docs/dd-061-interpreter-performance-roadmap.md
Original file line number Diff line number Diff line change
Expand Up @@ -326,31 +326,33 @@ Acceptance criteria:

Candidate benchmark note: local 3s/3-run `wrk` pass at 16 connections/2 threads, comparing `origin/main` at `52554e9` to this PR's dev-release binary, showed `/template/layout` RPS `23807.29 -> 24701.15` (+3.8%) and `/template/rows` RPS `6053.89 -> 6420.77` (+6.1%), while non-template routes stayed within noise.

### PR 5: Direct native/global call fast path
### PR 5: Targeted native/global call fast path

**Goal:** make common function calls cheaper without changing language semantics.

The interpreter already snapshots `builtin_bindings`, but ordinary identifier call evaluation still goes through generic expression evaluation and `Value::NativeFunction` call handling. We should add a constrained fast path for simple identifier calls where the callee is a stable native/global function.
The interpreter already snapshots `builtin_bindings`, but ordinary identifier call evaluation still goes through generic expression evaluation and clones argument values before invoking `Value::NativeFunction`. A broad direct-call path for every stable native/global call is not automatically a win: it can spend more time proving the callee is unshadowed than it saves. The useful first slice is narrower and measurable: `len(identifier)` should not clone large arrays/maps just to compute their length.

Scope:

- [ ] Profile common native calls in template/page workloads (`len`, string helpers, collections helpers, response builders, template filters).
- [ ] Add a direct-call path for `Expression::Call { function: Identifier(name), ... }` after server-action/template/fs special cases are handled.
- [ ] Avoid re-looking-up stable native functions through recursive environment chains when the name is known to be an unshadowed builtin/prelude binding.
- [ ] Preserve user-defined shadowing behavior. If an app defines `len`, the app binding must win.
- [ ] Add tests for shadowing, imported functions, prelude functions, and ordinary user functions.
- [x] Profile common native calls in template/page workloads (`len`, string helpers, collections helpers, response builders, template filters).
- [x] Add a targeted fast path for `len(identifier)` after preserving normal callee shadowing semantics.
- [x] Avoid cloning large identifier-bound arrays/maps when only their length is needed.
- [x] Preserve user-defined shadowing behavior. If an app defines `len`, the app binding must win.
- [x] Add tests for builtin behavior, shadowing, type errors, and parent-scope lookup behavior.

Likely files:

- `src/interpreter.rs`
- possibly a small call-dispatch helper to reduce the existing special-case ladder
- focused interpreter tests
- `examples/perf/*` and benchmark harness docs for the native-call fixture

Acceptance criteria:

- [ ] No behavior change for shadowing/imports.
- [ ] Simple native-call benchmark improves.
- [ ] Call dispatch code reads cleaner after the change, not more haunted.
- [x] No behavior change for shadowing/imports.
- [x] Simple native-call benchmark improves.
- [x] Call dispatch code stays localized rather than more haunted.

Candidate benchmark note: local 3s/3-run `wrk` pass at 16 connections/2 threads, comparing `origin/main` plus the new `/native/calls` fixture to this PR's dev-release binary, showed `/native/calls` RPS `349.95 -> 1574.70` (+350.0%). Other routes stayed within normal local noise.

### PR 6: Environment lookup measurement + low-risk lookup cache

Expand Down Expand Up @@ -505,7 +507,7 @@ Recommended local toolchain:
- [x] PR 2 adds a repeatable benchmark harness and baseline results.
- [x] PR 3 makes ordinary `template(path, data)` use a safe automatic AST/cache path.
- [x] PR 4 reduces template render/loop scope overhead without semantic drift.
- [ ] PR 5 adds a safe direct native/global call fast path, or documents why profiling does not justify it.
- [x] PR 5 adds a safe targeted native/global call fast path for `len(identifier)`.
- [ ] PR 6 adds lookup instrumentation/cache only if measurements justify it.
- [ ] PR 7 cleans request/response allocation only if profiles show meaningful headroom.
- [ ] DD-061 is updated after each merged PR with measured deltas and completed checkboxes.
Expand All @@ -515,6 +517,4 @@ Recommended local toolchain:

## Current Recommendation

With the automatic template AST cache and loop-scope cleanup complete, use benchmark data to decide whether **PR 5: direct native/global call fast path** is justified next. Start by profiling common native calls in template/page workloads and only add the fast path if shadowing/import semantics can stay obvious and well-tested.

If PR 5 does not produce a clean measurable win, skip to lookup instrumentation rather than forcing a dispatch optimization. Computers are annoyingly literal; we should let them tell us where they hurt.
With the automatic template AST cache, loop-scope cleanup, and targeted `len(identifier)` fast path complete, use benchmark/profile data to choose between **PR 6: environment lookup measurement/cache** and **PR 7: request/response allocation cleanup**. Do not force a broad generic native-call shortcut unless a profile shows it beats the extra shadowing checks.
1 change: 1 addition & 0 deletions examples/perf/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ The DB fixtures use simple `SELECT` queries so they do not require schema setup.
- `/json` — small JSON response
- `/param/{id}` — route params and map reads
- `/compute` — interpreter loop inside an HTTP request
- `/native/calls` — repeated stable native/global calls (`len`, `str`) inside an HTTP request
- `/template/layout` — external template render with layout, partial, and loop
- `/template/rows` — template-heavy 100-row render
- `/db/single` — optional single PostgreSQL query
Expand Down
11 changes: 11 additions & 0 deletions examples/perf/server.tnt
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,16 @@ fn compute_loop(req: Map) -> Map {
return json(map { "total": total })
}

fn native_call_loop(req: Map) -> Map {
let mut total = 0
let label = "ntnt-performance"
let rows = ROWS_10
for i in 0..5000 {
total = total + len(label) + len(rows) + len(str(i))
}
return json(map { "total": total })
}

fn template_layout(req: Map) -> Map {
let rendered = template("views/page.html", map {
"title": "Template layout benchmark",
Expand Down Expand Up @@ -121,6 +131,7 @@ get("/", plaintext)
get("/json", small_json)
get("/param/{id}", route_param)
get("/compute", compute_loop)
get("/native/calls", native_call_loop)
get("/template/layout", template_layout)
get("/template/rows", template_rows)
get("/db/single", db_single)
Expand Down
1 change: 1 addition & 0 deletions scripts/bench/run-benchmarks.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
{"name": "small JSON route", "path": "/json"},
{"name": "route param + map read", "path": "/param/12345"},
{"name": "compute loop route", "path": "/compute"},
{"name": "native call loop route", "path": "/native/calls"},
{"name": "template layout + partial", "path": "/template/layout"},
{"name": "template 100-row loop", "path": "/template/rows"},
]
Expand Down
160 changes: 155 additions & 5 deletions src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,11 +399,48 @@ impl Environment {

pub fn get(&self, name: &str) -> Option<Value> {
if let Some(value) = self.values.get(name) {
Some(value.clone())
} else if let Some(ref parent) = self.parent {
parent.borrow().get(name)
} else {
None
return Some(value.clone());
}

let mut next = self.parent.clone();
while let Some(env_rc) = next {
let env = env_rc.borrow();
if let Some(value) = env.values.get(name) {
return Some(value.clone());
}
next = env.parent.clone();
}

None
}

fn len_of_binding(&self, name: &str) -> Option<Result<Value>> {
if let Some(value) = self.values.get(name) {
return Some(Self::len_of_value(value));
}

let mut next = self.parent.clone();
while let Some(env_rc) = next {
let env = env_rc.borrow();
if let Some(value) = env.values.get(name) {
return Some(Self::len_of_value(value));
}
next = env.parent.clone();
}

None
Comment thread
greptile-apps[bot] marked this conversation as resolved.
}

fn len_of_value(value: &Value) -> Result<Value> {
match value {
Value::String(s) => Ok(Value::Int(s.len() as i64)),
Value::Array(a) => Ok(Value::Int(a.len() as i64)),
Value::Map(m) => Ok(Value::Int(m.len() as i64)),
other => Err(IntentError::type_error_with_context(
format!("len() requires a collection, got {}", other.type_name()),
TypeContext::new("String, Array, or Map", other.type_name())
.with_hint("Use type(x) to check the type before calling len()"),
)),
}
}

Expand Down Expand Up @@ -5930,6 +5967,24 @@ impl Interpreter {
} => {
// Special handling for old() in postconditions
if let Expression::Identifier(name) = function.as_ref() {
// Fast path for len(identifier): avoid cloning large arrays/maps just to
// compute their length. This preserves normal function shadowing by only
// firing when `len` currently resolves to the builtin native function.
if name == "len" && arguments.len() == 1 {
if let Expression::Identifier(arg_name) = &arguments[0] {
let len_is_builtin = matches!(
self.environment.borrow().get("len"),
Some(Value::NativeFunction { name, .. }) if name == "len"
);
if len_is_builtin {
let len_result = self.environment.borrow().len_of_binding(arg_name);
if let Some(result) = len_result {
return result;
}
}
}
}

if name == "old" && arguments.len() == 1 {
// Look up the pre-execution value
let key = format!("{:?}", &arguments[0]);
Expand Down Expand Up @@ -10747,6 +10802,101 @@ mod tests {
))
}

#[test]
fn environment_get_finds_parent_bindings_and_preserves_child_shadowing() {
let root = Rc::new(RefCell::new(Environment::new()));
root.borrow_mut()
.define("name".to_string(), Value::String("root".to_string()));
root.borrow_mut()
.define("only_root".to_string(), Value::Int(7));

let child = Environment::with_parent(Rc::clone(&root));
assert!(matches!(child.get("only_root"), Some(Value::Int(7))));
assert!(matches!(child.get("name"), Some(Value::String(ref s)) if s == "root"));

let mut shadowing_child = Environment::with_parent(root);
shadowing_child.define("name".to_string(), Value::String("child".to_string()));
assert!(matches!(shadowing_child.get("name"), Some(Value::String(ref s)) if s == "child"));
assert!(shadowing_child.get("missing").is_none());
}

#[test]
fn environment_get_handles_deep_scope_chains_iteratively() {
let root = Rc::new(RefCell::new(Environment::new()));
root.borrow_mut()
.define("target".to_string(), Value::Int(42));

let mut current = root;
for _ in 0..64 {
current = Rc::new(RefCell::new(Environment::with_parent(current)));
}

assert!(matches!(
current.borrow().get("target"),
Some(Value::Int(42))
));
}

#[test]
fn environment_len_of_binding_handles_parent_scopes() {
let root = Rc::new(RefCell::new(Environment::new()));
root.borrow_mut().define(
"rows".to_string(),
Value::Array(vec![Value::Int(1), Value::Int(2), Value::Int(3)]),
);

let child = Environment::with_parent(Rc::new(RefCell::new(Environment::with_parent(root))));
assert!(matches!(
child.len_of_binding("rows"),
Some(Ok(Value::Int(3)))
));
}

#[test]
fn len_identifier_fast_path_preserves_builtin_behavior() {
let result = eval(
r#"
let rows = [1, 2, 3, 4]
len(rows)
"#,
)
.unwrap();

assert!(matches!(result, Value::Int(4)));
}

#[test]
fn len_identifier_fast_path_preserves_shadowed_len_function() {
let result = eval(
r#"
fn len(value) {
return 99
}

let rows = [1, 2, 3, 4]
len(rows)
"#,
)
.unwrap();

assert!(matches!(result, Value::Int(99)));
}

#[test]
fn len_identifier_fast_path_preserves_type_errors() {
let result = eval(
r#"
let n = 123
len(n)
"#,
)
.unwrap_err();

assert!(
matches!(result, IntentError::TypeError { message, .. } if message.contains("len() requires a collection"))
);
}

#[test]
fn template_cache_reuses_parsed_templates_and_partials() {
let dir = unique_template_test_dir("reuse");
Expand Down
Loading