Per-command timings for script runs and batches (#1868)

Thanks @DuBearte.

crates/mcp/src/backend.rs

@@ -21,6 +21,11 @@ pub trait Backend {
     fn design_path(&self) -> Option<String> {
         None
     }
+    /// Commands run in this process, on this thread (the kernel's time in them can be measured
+    /// here, #1833).
+    fn in_process(&self) -> bool {
+        false
+    }
 }
 
 /// Longest reply line accepted from the app.
@@ -308,4 +313,7 @@ impl Backend for Headless {
     fn design_path(&self) -> Option<String> {
         self.session.path.clone()
     }
+    fn in_process(&self) -> bool {
+        true
+    }
 }

crates/mcp/src/tests.rs

@@ -1479,3 +1479,44 @@ fn measure_items_gives_diameters_and_centre_distances() {
     assert!((two["min"].as_f64().unwrap() - 45.0).abs() < 1e-9 && (two["max"].as_f64().unwrap() - 55.0).abs() < 1e-9, "{two}");
     assert!(two["distance_mm"].is_number() && two["from"].is_array(), "{two}");
 }
+
+/// #1833: `timings` / `profile` time every step run, the failing one included, headless with the
+/// kernel's share; bridged, the app is asked to time them and its numbers are passed on.
+#[test]
+fn batch_reports_timings_and_profile() {
+    let mut s = server();
+    let steps = json!([
+        {"command": "solid.box", "params": {"length": 10, "width": 10, "height": 10}},
+        {"command": "solid.box", "params": {"length": 4, "width": 4, "height": 30, "operation": "join"}},
+    ]);
+    let r = raw_tool(&mut s, "batch", json!({"commands": steps, "profile": true}));
+    let body: Value = serde_json::from_str(r["result"]["content"][0]["text"].as_str().unwrap()).unwrap();
+    let rows = body["timings"].as_array().unwrap();
+    assert_eq!(rows.len(), 2, "{body}");
+    assert_eq!((rows[1]["index"].as_u64(), rows[1]["command"].as_str()), (Some(1), Some("solid.box")));
+    assert!(rows[1]["profile"]["boolean_ms"].as_f64().unwrap() > 0.0, "{body}");
+    assert!(body["total_ms"].as_f64().is_some(), "{body}");
+
+    let r = raw_tool(
+        &mut s,
+        "batch",
+        json!({"timings": true, "commands": [{"command": "solid.box", "params": {"length": 1, "width": 1, "height": 1}}, {"command": "NoSuchCommand"}]}),
+    );
+    let body: Value = serde_json::from_str(r["result"]["content"][0]["text"].as_str().unwrap()).unwrap();
+    assert_eq!(body["timings"].as_array().map(Vec::len), Some(2), "the failing step is timed too: {body}");
+    assert!(body["timings"][0].get("profile").is_none());
+    // Not asked: no timings.
+    let d = tool(&mut s, "batch", json!({"commands": [{"command": "solid.box", "params": {"length": 1, "width": 1, "height": 1}}]}));
+    assert!(d.get("timings").is_none(), "{d}");
+
+    let reply =
+        json!({"completed": 1, "results": [{"x": 1}], "undone": 0, "timings": [{"index": 0, "command": "a.one", "ms": 2.5}], "total_ms": 2.6});
+    let (b, body, _) = recorded_batch(Ok(reply), json!({"commands": [{"command": "a.one"}], "profile": true}));
+    assert_eq!(b.calls[0].1["timings"], true);
+    assert_eq!(b.calls[0].1["profile"], true);
+    assert_eq!((body["timings"][0]["ms"].as_f64(), body["total_ms"].as_f64()), (Some(2.5), Some(2.6)), "{body}");
+    // An app without engine.batch is timed step by step here, without a kernel split it can't measure.
+    let (_, body, _) = recorded_batch(Err("unknown method `engine.batch`".into()), json!({"commands": [{"command": "a.one"}], "profile": true}));
+    assert_eq!(body["timings"][0]["command"], "a.one", "{body}");
+    assert!(body["timings"][0].get("profile").is_none(), "{body}");
+}

crates/mcp/src/tools.rs

@@ -5,6 +5,7 @@ use std::sync::atomic::{AtomicU64, Ordering};
 
 use serde_json::{Map, Value, json};
 use solidcraft_engine::io::vfs;
+use solidcraft_engine::timing;
 
 use crate::backend::Backend;
 
@@ -109,6 +110,8 @@ pub fn tool_definitions() -> Vec<Value> {
                         "items": {"type": "object", "properties": {"command": {"type": "string"}, "params": {"type": "object"}}, "required": ["command"]},
                     },
                     "rollback": {"type": "boolean", "description": "Undo completed steps when one fails (default true)"},
+                    "timings": {"type": "boolean", "description": "Add `timings`: [{index, command, ms}] for every step run (the failing one included) and `total_ms` (default false)"},
+                    "profile": {"type": "boolean", "description": "Like timings, and each step's `profile`: {boolean_ms, tessellate_ms, other_ms}, its time in kernel booleans, in tessellation and the rest (default false)"},
                     "verbose": {"type": "boolean", "description": "false: a compact reply. Feature steps leave out the full body list (created_bodies, changed_bodies and removed_bodies stay), sketch steps the ids they repeat outside `created` (default true)"},
                 }),
                 &["commands"],
@@ -496,10 +499,16 @@ fn batch(b: &mut dyn Backend, a: &Map<String, Value>) -> Value {
     }
     let rollback = a.get("rollback").and_then(Value::as_bool).unwrap_or(true);
     let verbose = a.get("verbose").and_then(Value::as_bool).unwrap_or(true);
+    let profile = a.get("profile").and_then(Value::as_bool).unwrap_or(false);
+    let timed = profile || a.get("timings").and_then(Value::as_bool).unwrap_or(false);
     // An app that runs batches itself answers the whole batch in one request, so one frame
     // (#743). One too long for a control line goes step by step: the app would refuse it unrun.
     // The margin covers the envelope and the `allow_paths` the backend adds.
-    let params = json!({"commands": steps, "rollback": rollback});
+    let mut params = json!({"commands": steps, "rollback": rollback});
+    if timed && let Some(o) = params.as_object_mut() {
+        o.insert("timings".into(), Value::Bool(true));
+        o.insert("profile".into(), Value::Bool(profile));
+    }
     if params.to_string().len() + 64 * 1024 <= solidcraft_engine::MAX_LINE {
         match b.call("engine.batch", params) {
             Ok(r) => return batch_reply(&r, rollback, verbose),
@@ -516,42 +525,57 @@ fn batch(b: &mut dyn Backend, a: &Map<String, Value>) -> Value {
     }
     let start = if rollback { Undo::begin(b) } else { Undo::None };
     let mut results = Vec::with_capacity(steps.len());
+    let mut rows = Vec::new();
+    let clock = timing::Clock::start();
+    // The kernel's share can only be measured where the commands run: in this process.
+    let profile = profile && b.in_process();
     for (i, step) in steps.iter().enumerate() {
         let command = step.get("command").or_else(|| step.get("id")).and_then(Value::as_str);
-        let r = match command {
+        let (r, took) = timing::time(profile, || match command {
             Some(id) => exec(b, id, step.get("params").cloned().filter(|p| !p.is_null()).unwrap_or(json!({}))),
             None => Err("missing `command`".to_string()),
-        };
+        });
+        if timed {
+            rows.push(took.to_json(i, command.unwrap_or_default()));
+        }
         match r {
             Ok(v) => results.push(if verbose { v } else { compact(v) }),
             Err(e) => {
                 let undone = start.rollback(b, i);
-                return text_result(
-                    &json!({
-                        "ok": false,
-                        "completed": i,
-                        "failed": {"index": i, "command": command, "error": e},
-                        "rolled_back": rollback,
-                        "undone": undone,
-                        "results": results,
-                    }),
-                    true,
-                );
+                let reply = json!({
+                    "ok": false,
+                    "completed": i,
+                    "failed": {"index": i, "command": command, "error": e},
+                    "rolled_back": rollback,
+                    "undone": undone,
+                    "results": results,
+                });
+                return text_result(&with_timings(reply, timed, rows, clock.ms()), true);
             }
         }
     }
     start.finish(b);
-    text_result(&json!({"ok": true, "completed": results.len(), "results": results}), false)
+    let reply = json!({"ok": true, "completed": results.len(), "results": results});
+    text_result(&with_timings(reply, timed, rows, clock.ms()), false)
+}
+
+/// `reply` with `timings` and `total_ms` when they were asked for.
+fn with_timings(mut reply: Value, timed: bool, rows: Vec<Value>, total_ms: f64) -> Value {
+    if timed && let Some(o) = reply.as_object_mut() {
+        o.insert("timings".into(), Value::Array(rows));
+        o.insert("total_ms".into(), json!(timing::round_ms(total_ms)));
+    }
+    reply
 }
 
 /// The `batch` reply from the app's `engine.batch` answer, shaped as the per-step path shapes it.
 fn batch_reply(r: &Value, rollback: bool, verbose: bool) -> Value {
     let results: Vec<Value> =
         r["results"].as_array().cloned().unwrap_or_default().into_iter().map(|v| if verbose { v } else { compact(v) }).collect();
-    match r.get("failed").filter(|f| !f.is_null()) {
-        None => text_result(&json!({"ok": true, "completed": results.len(), "results": results}), false),
-        Some(failed) => text_result(
-            &json!({
+    let (mut reply, error) = match r.get("failed").filter(|f| !f.is_null()) {
+        None => (json!({"ok": true, "completed": results.len(), "results": results}), false),
+        Some(failed) => (
+            json!({
                 "ok": false,
                 "completed": r["completed"],
                 "failed": failed,
@@ -561,7 +585,16 @@ fn batch_reply(r: &Value, rollback: bool, verbose: bool) -> Value {
             }),
             true,
         ),
+    };
+    // The app timed the steps where they ran.
+    if let Some(o) = reply.as_object_mut() {
+        for k in ["timings", "total_ms"] {
+            if let Some(v) = r.get(k).filter(|v| !v.is_null()) {
+                o.insert(k.into(), v.clone());
+            }
+        }
     }
+    text_result(&reply, error)
 }
 
 /// A command's result without what the design already had (`execute` or `batch` with `verbose: false`): a