From 95221ca396832144030487733d620cbab702c943 Mon Sep 17 00:00:00 2001 From: Matt Keeter Date: Mon, 29 Apr 2024 08:42:43 -0400 Subject: [PATCH] Improve webworker architecture (#98) - Move script evaluation into worker, to avoid blocking the UI thread - Add evaluation limits to `Engine` - Use transfers to move buffers (instead of copying them) --- fidget/src/rhai/mod.rs | 11 +++++++++++ wasm-demo/index.ts | 42 ++++++++++++++++++------------------------ wasm-demo/message.ts | 28 ++++++++++++++++++++++++++-- wasm-demo/src/lib.rs | 1 + wasm-demo/worker.ts | 33 ++++++++++++++++++++++++++++++++- 5 files changed, 88 insertions(+), 27 deletions(-) diff --git a/fidget/src/rhai/mod.rs b/fidget/src/rhai/mod.rs index e9ad2735..87e9988e 100644 --- a/fidget/src/rhai/mod.rs +++ b/fidget/src/rhai/mod.rs @@ -141,6 +141,17 @@ impl Engine { Self { engine, context } } + /// Sets the operation limit (e.g. for untrusted scripts) + pub fn set_limit(&mut self, limit: u64) { + self.engine.on_progress(move |count| { + if count > limit { + Some("script runtime exceeded".into()) + } else { + None + } + }); + } + /// Executes a full script pub fn run(&mut self, script: &str) -> Result { self.context.lock().unwrap().clear(); diff --git a/wasm-demo/index.ts b/wasm-demo/index.ts index b35fe012..e1837442 100644 --- a/wasm-demo/index.ts +++ b/wasm-demo/index.ts @@ -6,10 +6,12 @@ import { defaultKeymap } from "@codemirror/commands"; import { ResponseKind, + ScriptRequest, ShapeRequest, StartRequest, - WorkerResponse, + ScriptResponse, WorkerRequest, + WorkerResponse, } from "./message"; import { RENDER_SIZE, WORKERS_PER_SIDE, WORKER_COUNT } from "./constants"; @@ -54,28 +56,7 @@ class App { } onScriptChanged(text: string) { - let shape = null; - let result = "Ok(..)"; - try { - shape = fidget.eval_script(text); - } catch (error) { - // Do some string formatting to make errors cleaner - result = error - .toString() - .replace("Rhai evaluation error: ", "Rhai evaluation error:\n") - .replace(" (line ", "\n(line ") - .replace(" (expecting ", "\n(expecting "); - } - this.output.setText(result); - - if (shape) { - const tape = fidget.serialize_into_tape(shape); - this.start_time = performance.now(); - this.workers_done = 0; - this.workers.forEach((w) => { - w.postMessage(new ShapeRequest(tape)); - }); - } + this.workers[0].postMessage(new ScriptRequest(text)); } onWorkerMessage(i: number, req: WorkerResponse) { @@ -103,6 +84,19 @@ class App { } break; } + case ResponseKind.Script: { + let r = req as ScriptResponse; + this.output.setText(r.output); + if (r.tape) { + this.start_time = performance.now(); + this.workers_done = 0; + this.workers.forEach((w) => { + w.postMessage(new ShapeRequest(r.tape)); + }); + } + break; + } + default: { console.error(`unknown worker req ${req}`); } @@ -131,7 +125,7 @@ class Editor { window.clearTimeout(this.timeout); } const text = v.state.doc.toString(); - this.timeout = window.setTimeout(() => cb(text), 500); + this.timeout = window.setTimeout(() => cb(text), 250); } }), ], diff --git a/wasm-demo/message.ts b/wasm-demo/message.ts index 6b0182b1..374dfeab 100644 --- a/wasm-demo/message.ts +++ b/wasm-demo/message.ts @@ -1,8 +1,19 @@ export enum RequestKind { + Script, Start, Shape, } +export class ScriptRequest { + kind: RequestKind.Script; + script: string; + + constructor(script: string) { + this.script = script; + this.kind = RequestKind.Script; + } +} + export class StartRequest { kind: RequestKind.Start; index: number; @@ -23,12 +34,13 @@ export class ShapeRequest { } } -export type WorkerRequest = ShapeRequest | StartRequest; +export type WorkerRequest = ScriptRequest | ShapeRequest | StartRequest; //////////////////////////////////////////////////////////////////////////////// export enum ResponseKind { Started, + Script, Image, } @@ -40,6 +52,18 @@ export class StartedResponse { } } +export class ScriptResponse { + kind: ResponseKind.Script; + output: string; + tape: Uint8Array | null; + + constructor(output: string, tape: Uint8Array | null) { + this.output = output; + this.tape = tape; + this.kind = ResponseKind.Script; + } +} + export class ImageResponse { kind: ResponseKind.Image; data: Uint8Array; @@ -50,4 +74,4 @@ export class ImageResponse { } } -export type WorkerResponse = StartedResponse | ImageResponse; +export type WorkerResponse = StartedResponse | ScriptResponse | ImageResponse; diff --git a/wasm-demo/src/lib.rs b/wasm-demo/src/lib.rs index 63d4e2e0..41f0b67a 100644 --- a/wasm-demo/src/lib.rs +++ b/wasm-demo/src/lib.rs @@ -19,6 +19,7 @@ pub struct JsVmShape(VmShape); #[wasm_bindgen] pub fn eval_script(s: &str) -> Result { let mut engine = fidget::rhai::Engine::new(); + engine.set_limit(50_000); // ¯\_(ツ)_/¯ let out = engine.eval(s); out.map(JsTree).map_err(|e| format!("{e}")) } diff --git a/wasm-demo/worker.ts b/wasm-demo/worker.ts index ba21e752..0c53f701 100644 --- a/wasm-demo/worker.ts +++ b/wasm-demo/worker.ts @@ -1,6 +1,8 @@ import { ImageResponse, RequestKind, + ScriptRequest, + ScriptResponse, ShapeRequest, StartRequest, StartedResponse, @@ -27,7 +29,32 @@ class Worker { this.index, WORKERS_PER_SIDE, ); - postMessage(new ImageResponse(out)); + postMessage(new ImageResponse(out), { transfer: [out.buffer] }); + } + + run(s: ScriptRequest) { + let shape = null; + let result = "Ok(..)"; + try { + shape = fidget.eval_script(s.script); + } catch (error) { + // Do some string formatting to make errors cleaner + result = error + .toString() + .replace("Rhai evaluation error: ", "Rhai evaluation error:\n") + .replace(" (line ", "\n(line ") + .replace(" (expecting ", "\n(expecting "); + } + + let tape = null; + if (shape) { + tape = fidget.serialize_into_tape(shape); + postMessage(new ScriptResponse(result, tape), { + transfer: [tape.buffer], + }); + } else { + postMessage(new ScriptResponse(result, tape)); + } } } @@ -46,6 +73,10 @@ async function run() { worker!.render(req as ShapeRequest); break; } + case RequestKind.Script: { + worker!.run(req as ScriptRequest); + break; + } default: console.error(`unknown worker request ${req}`); }