From 83bece56b01f6997cb71e9289a4d83a398cde0c8 Mon Sep 17 00:00:00 2001 From: Aaron O'Mullan Date: Sun, 25 Apr 2021 22:00:05 +0200 Subject: [PATCH] refactor(core): move op cache sync responsibility to rust space (#10340) Even if bootstrapping the JS runtime is low level, it's an abstraction leak of core to require users to call `Deno.core.ops()` in JS space. So instead we're introducing a `JsRuntime::sync_ops_cache()` method, once we have runtime extensions a new runtime will ensure the ops cache is setup (for the provided extensions) and then loading/unloading plugins should be the only operations that require op cache syncs --- bench_util/src/js_runtime.rs | 8 +------- cli/build.rs | 2 ++ cli/lsp/tsc.rs | 1 + cli/main.rs | 2 ++ cli/tsc.rs | 1 + cli/tsc/99_main_compiler.js | 5 ----- core/benches/op_baseline.rs | 11 +---------- core/core.js | 9 ++++++--- core/examples/hello_world.rs | 7 ++----- core/examples/http_bench_json_ops.js | 2 -- core/examples/http_bench_json_ops.rs | 1 + core/ops_json.rs | 12 +++++------- core/runtime.rs | 8 ++++++-- op_crates/url/benches/url_ops.rs | 9 +-------- runtime/js/99_main.js | 2 -- runtime/web_worker.rs | 1 + runtime/worker.rs | 1 + 17 files changed, 31 insertions(+), 51 deletions(-) diff --git a/bench_util/src/js_runtime.rs b/bench_util/src/js_runtime.rs index 415f4a135c..d5509d624a 100644 --- a/bench_util/src/js_runtime.rs +++ b/bench_util/src/js_runtime.rs @@ -16,13 +16,7 @@ pub fn create_js_runtime(setup: impl FnOnce(&mut JsRuntime)) -> JsRuntime { setup(&mut rt); // Init ops - rt.execute( - "init", - r#" - Deno.core.ops(); - "#, - ) - .unwrap(); + rt.sync_ops_cache(); rt } diff --git a/cli/build.rs b/cli/build.rs index 930ba376f1..d4e6a92aba 100644 --- a/cli/build.rs +++ b/cli/build.rs @@ -210,6 +210,8 @@ fn create_compiler_snapshot( } }), ); + js_runtime.sync_ops_cache(); + create_snapshot(js_runtime, snapshot_path, files); } diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index fef6053976..bb9311b2f4 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -2122,6 +2122,7 @@ pub fn start(debug: bool) -> Result { runtime.register_op("op_respond", op(respond)); runtime.register_op("op_script_names", op(script_names)); runtime.register_op("op_script_version", op(script_version)); + runtime.sync_ops_cache(); let init_config = json!({ "debug": debug }); let init_src = format!("globalThis.serverInit({});", init_config); diff --git a/cli/main.rs b/cli/main.rs index 0005bb891d..c42604146f 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -148,6 +148,7 @@ fn create_web_worker_callback( if args.use_deno_namespace { ops::runtime_compiler::init(js_runtime); } + js_runtime.sync_ops_cache(); } worker.bootstrap(&options); @@ -218,6 +219,7 @@ pub fn create_main_worker( // above ops::errors::init(js_runtime); ops::runtime_compiler::init(js_runtime); + js_runtime.sync_ops_cache(); } worker.bootstrap(&options); diff --git a/cli/tsc.rs b/cli/tsc.rs index 0fac29ce3d..167b5b1105 100644 --- a/cli/tsc.rs +++ b/cli/tsc.rs @@ -496,6 +496,7 @@ pub fn exec(request: Request) -> Result { runtime.register_op("op_load", op(load)); runtime.register_op("op_resolve", op(resolve)); runtime.register_op("op_respond", op(respond)); + runtime.sync_ops_cache(); let startup_source = "globalThis.startup({ legacyFlag: false })"; let request_value = json!({ diff --git a/cli/tsc/99_main_compiler.js b/cli/tsc/99_main_compiler.js index ebb4d679d5..f944b21b8d 100644 --- a/cli/tsc/99_main_compiler.js +++ b/cli/tsc/99_main_compiler.js @@ -778,7 +778,6 @@ delete Object.prototype.__proto__; } hasStarted = true; languageService = ts.createLanguageService(host); - core.ops(); setLogDebug(debugFlag, "TSLS"); debug("serverInit()"); } @@ -793,13 +792,9 @@ delete Object.prototype.__proto__; throw new Error("The compiler runtime already started."); } hasStarted = true; - core.ops(); setLogDebug(!!debugFlag, "TS"); } - // Setup the compiler runtime during the build process. - core.ops(); - // A build time only op that provides some setup information that is used to // ensure the snapshot is setup properly. /** @type {{ buildSpecifier: string; libs: string[] }} */ diff --git a/core/benches/op_baseline.rs b/core/benches/op_baseline.rs index a7f3bda1df..c0c988eb6d 100644 --- a/core/benches/op_baseline.rs +++ b/core/benches/op_baseline.rs @@ -20,16 +20,7 @@ fn create_js_runtime() -> JsRuntime { runtime.register_op("nop", |state, _, _| { Op::Sync(serialize_op_result(Ok(9), state)) }); - - // Init ops - runtime - .execute( - "init", - r#" - Deno.core.ops(); - "#, - ) - .unwrap(); + runtime.sync_ops_cache(); runtime } diff --git a/core/core.js b/core/core.js index a11f281f1a..fd9b2c3ea3 100644 --- a/core/core.js +++ b/core/core.js @@ -60,12 +60,14 @@ } function ops() { - // op id 0 is a special value to retrieve the map of registered ops. - const newOpsCache = Object.fromEntries(opcall(0)); - opsCache = Object.freeze(newOpsCache); return opsCache; } + function syncOpsCache() { + // op id 0 is a special value to retrieve the map of registered ops. + opsCache = Object.freeze(Object.fromEntries(opcall(0))); + } + function handleAsyncMsgFromRust() { for (let i = 0; i < arguments.length; i += 2) { const promiseId = arguments[i]; @@ -130,5 +132,6 @@ resources, registerErrorClass, handleAsyncMsgFromRust, + syncOpsCache, }); })(this); diff --git a/core/examples/hello_world.rs b/core/examples/hello_world.rs index 62d2683401..a9d2934f64 100644 --- a/core/examples/hello_world.rs +++ b/core/examples/hello_world.rs @@ -56,6 +56,7 @@ fn main() { Ok(sum) }), ); + runtime.sync_ops_cache(); // Now we see how to invoke the ops we just defined. The runtime automatically // contains a Deno.core object with several functions for interacting with it. @@ -64,11 +65,7 @@ fn main() { .execute( "", r#" -// First we initialize the ops cache. -// This maps op names to their id's. -Deno.core.ops(); - -// Then we define a print function that uses +// Define a print function that uses // our op_print op to display the stringified argument. const _newline = new Uint8Array([10]); function print(value) { diff --git a/core/examples/http_bench_json_ops.js b/core/examples/http_bench_json_ops.js index 6727471969..ad36dd674a 100644 --- a/core/examples/http_bench_json_ops.js +++ b/core/examples/http_bench_json_ops.js @@ -54,8 +54,6 @@ async function serve(rid) { } async function main() { - Deno.core.ops(); - const listenerRid = listen(); Deno.core.print(`http_bench_ops listening on http://127.0.0.1:4544/\n`); diff --git a/core/examples/http_bench_json_ops.rs b/core/examples/http_bench_json_ops.rs index e1b435e4ce..f891caa9eb 100644 --- a/core/examples/http_bench_json_ops.rs +++ b/core/examples/http_bench_json_ops.rs @@ -116,6 +116,7 @@ fn create_js_runtime() -> JsRuntime { runtime.register_op("accept", deno_core::op_async(op_accept)); runtime.register_op("read", deno_core::op_async(op_read)); runtime.register_op("write", deno_core::op_async(op_write)); + runtime.sync_ops_cache(); runtime } diff --git a/core/ops_json.rs b/core/ops_json.rs index 309fac12d9..a04bdac609 100644 --- a/core/ops_json.rs +++ b/core/ops_json.rs @@ -25,15 +25,15 @@ use std::rc::Rc; /// ```ignore /// let mut runtime = JsRuntime::new(...); /// runtime.register_op("hello", deno_core::op_sync(Self::hello_op)); +/// runtime.sync_ops_cache(); /// ``` /// /// ...it can be invoked from JS using the provided name, for example: /// ```js -/// Deno.core.ops(); /// let result = Deno.core.opSync("function_name", args); /// ``` /// -/// The `Deno.core.ops()` statement is needed once before any op calls, for initialization. +/// `runtime.sync_ops_cache()` must be called after registering new ops /// A more complete example is available in the examples directory. pub fn op_sync(op_fn: F) -> Box where @@ -63,15 +63,15 @@ where /// ```ignore /// let mut runtime = JsRuntime::new(...); /// runtime.register_op("hello", deno_core::op_async(Self::hello_op)); +/// runtime.sync_ops_cache(); /// ``` /// /// ...it can be invoked from JS using the provided name, for example: /// ```js -/// Deno.core.ops(); /// let future = Deno.core.opAsync("function_name", args); /// ``` /// -/// The `Deno.core.ops()` statement is needed once before any op calls, for initialization. +/// `runtime.sync_ops_cache()` must be called after registering new ops /// A more complete example is available in the examples directory. pub fn op_async(op_fn: F) -> Box where @@ -116,13 +116,11 @@ mod tests { } runtime.register_op("op_throw", op_async(op_throw)); + runtime.sync_ops_cache(); runtime .execute( "", r#" - // First we initialize the ops cache. This maps op names to their id's. - Deno.core.ops(); - async function f1() { await Deno.core.opAsync('op_throw', 'hello'); } diff --git a/core/runtime.rs b/core/runtime.rs index 34a83e3f3f..7475c70209 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -367,6 +367,11 @@ impl JsRuntime { state.js_recv_cb.replace(v8::Global::new(scope, cb)); } + /// Ensures core.js has the latest op-name to op-id mappings + pub fn sync_ops_cache(&mut self) { + self.execute("", "Deno.core.syncOpsCache()").unwrap() + } + /// Returns the runtime's op state, which can be used to maintain ops /// and access resources between op calls. pub fn op_state(&mut self) -> Rc> { @@ -2140,6 +2145,7 @@ pub mod tests { module_loader: Some(loader), ..Default::default() }); + runtime.sync_ops_cache(); runtime .execute( "file:///dyn_import3.js", @@ -2149,8 +2155,6 @@ pub mod tests { if (mod.b() !== 'b') { throw Error("bad"); } - // Now do any op - Deno.core.ops(); })(); "#, ) diff --git a/op_crates/url/benches/url_ops.rs b/op_crates/url/benches/url_ops.rs index 37efb0a339..7d5d32879b 100644 --- a/op_crates/url/benches/url_ops.rs +++ b/op_crates/url/benches/url_ops.rs @@ -15,6 +15,7 @@ fn create_js_runtime() -> JsRuntime { "op_url_stringify_search_params", op_sync(deno_url::op_url_stringify_search_params), ); + runtime.sync_ops_cache(); runtime .execute( @@ -23,14 +24,6 @@ fn create_js_runtime() -> JsRuntime { ) .unwrap(); deno_url::init(&mut runtime); - runtime - .execute( - "init", - r#" - Deno.core.ops(); - "#, - ) - .unwrap(); runtime .execute("setup", "const { URL } = globalThis.__bootstrap.url;") .unwrap(); diff --git a/runtime/js/99_main.js b/runtime/js/99_main.js index 851d798c3e..d2626a07d8 100644 --- a/runtime/js/99_main.js +++ b/runtime/js/99_main.js @@ -142,8 +142,6 @@ delete Object.prototype.__proto__; } function runtimeStart(runtimeOptions, source) { - core.ops(); - core.setMacrotaskCallback(timers.handleTimerMacrotask); version.setVersions( runtimeOptions.denoVersion, diff --git a/runtime/web_worker.rs b/runtime/web_worker.rs index cdc3d7e3d8..f6f88f59b3 100644 --- a/runtime/web_worker.rs +++ b/runtime/web_worker.rs @@ -284,6 +284,7 @@ impl WebWorker { t.add(stream); } } + js_runtime.sync_ops_cache(); worker } diff --git a/runtime/worker.rs b/runtime/worker.rs index 6de87f52ee..6dbf8e7ec8 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -171,6 +171,7 @@ impl MainWorker { t.add(stream); } } + js_runtime.sync_ops_cache(); worker }