From 283a563a5135d6b9be086da0521c75cb649a67c8 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Thu, 30 Sep 2021 06:15:00 +0000 Subject: [PATCH 1/4] feat(ext/ffi): Non-blocking FFI --- Cargo.lock | 1 + ext/ffi/00_ffi.js | 11 +++++- ext/ffi/Cargo.toml | 1 + ext/ffi/lib.rs | 60 ++++++++++++++++++++++------- test_ffi/src/lib.rs | 9 +++++ test_ffi/tests/integration_tests.rs | 4 ++ test_ffi/tests/test.js | 34 +++++++++++----- 7 files changed, 95 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e8284da3c780a7..d2ede9ba0c381b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -778,6 +778,7 @@ dependencies = [ "deno_core", "dlopen", "serde", + "tokio", ] [[package]] diff --git a/ext/ffi/00_ffi.js b/ext/ffi/00_ffi.js index 553ea1dd3663b3..4e61dc65a21bf0 100644 --- a/ext/ffi/00_ffi.js +++ b/ext/ffi/00_ffi.js @@ -13,8 +13,15 @@ this.#rid = core.opSync("op_ffi_load", { path, symbols }); for (const symbol in symbols) { - this.symbols[symbol] = (...parameters) => - core.opSync("op_ffi_call", { rid: this.#rid, symbol, parameters }); + this.symbols[symbol] = symbols[symbol].nonblocking + ? async (...parameters) => + core.opAsync("op_ffi_call_async", { + rid: this.#rid, + symbol, + parameters, + }) + : (...parameters) => + core.opSync("op_ffi_call", { rid: this.#rid, symbol, parameters }); } } diff --git a/ext/ffi/Cargo.toml b/ext/ffi/Cargo.toml index 398aa5995795c3..6d63575621686d 100644 --- a/ext/ffi/Cargo.toml +++ b/ext/ffi/Cargo.toml @@ -18,3 +18,4 @@ deno_core = { version = "0.101.0", path = "../../core" } dlopen = "0.1.8" libffi = { version = "=0.0.7", package = "deno-libffi" } serde = { version = "1.0.129", features = ["derive"] } +tokio = { version = "1.10.1", features = ["full"] } diff --git a/ext/ffi/lib.rs b/ext/ffi/lib.rs index 673f8347223f57..662b5ec3600fcd 100644 --- a/ext/ffi/lib.rs +++ b/ext/ffi/lib.rs @@ -3,6 +3,7 @@ use deno_core::error::bad_resource_id; use deno_core::error::AnyError; use deno_core::include_js_files; +use deno_core::op_async; use deno_core::op_sync; use deno_core::serde_json::json; use deno_core::serde_json::Value; @@ -14,6 +15,7 @@ use dlopen::raw::Library; use libffi::middle::Arg; use serde::Deserialize; use std::borrow::Cow; +use std::cell::RefCell; use std::collections::HashMap; use std::convert::TryFrom; use std::ffi::c_void; @@ -45,6 +47,7 @@ impl FfiPermissions for NoFfiPermissions { } } +#[derive(Clone)] struct Symbol { cif: libffi::middle::Cif, ptr: libffi::middle::CodePtr, @@ -52,6 +55,9 @@ struct Symbol { result_type: NativeType, } +unsafe impl Send for Symbol {} +unsafe impl Sync for Symbol {} + struct DynamicLibraryResource { lib: Library, symbols: HashMap, @@ -107,6 +113,7 @@ pub fn init(unstable: bool) -> Extension { .ops(vec![ ("op_ffi_load", op_sync(op_ffi_load::

)), ("op_ffi_call", op_sync(op_ffi_call)), + ("op_ffi_call_async", op_async(op_ffi_call_async)), ]) .state(move |state| { // Stolen from deno_webgpu, is there a better option? @@ -302,20 +309,7 @@ struct FfiCallArgs { parameters: Vec, } -fn op_ffi_call( - state: &mut deno_core::OpState, - args: FfiCallArgs, - _: (), -) -> Result { - let resource = state - .resource_table - .get::(args.rid)?; - - let symbol = resource - .symbols - .get(&args.symbol) - .ok_or_else(bad_resource_id)?; - +fn ffi_call(args: FfiCallArgs, symbol: &Symbol) -> Result { let native_values = symbol .parameter_types .iter() @@ -374,3 +368,41 @@ fn op_ffi_call( } }) } + +fn op_ffi_call( + state: &mut deno_core::OpState, + args: FfiCallArgs, + _: (), +) -> Result { + let resource = state + .resource_table + .get::(args.rid)?; + + let symbol = resource + .symbols + .get(&args.symbol) + .ok_or_else(bad_resource_id)?; + + ffi_call(args, symbol) +} + +/// A non-blocking FFI call. +async fn op_ffi_call_async( + state: Rc>, + args: FfiCallArgs, + _: (), +) -> Result { + let resource = state + .borrow() + .resource_table + .get::(args.rid)?; + let symbols = &resource.symbols; + let symbol = symbols + .get(&args.symbol) + .ok_or_else(bad_resource_id)? + .clone(); + + tokio::task::spawn_blocking(move || ffi_call(args, &symbol)) + .await + .unwrap() +} diff --git a/test_ffi/src/lib.rs b/test_ffi/src/lib.rs index a5a07795028b5d..c91d05e0554a8f 100644 --- a/test_ffi/src/lib.rs +++ b/test_ffi/src/lib.rs @@ -1,3 +1,6 @@ +use std::thread::sleep; +use std::time::Duration; + #[no_mangle] pub extern "C" fn print_something() { println!("something"); @@ -42,3 +45,9 @@ pub extern "C" fn add_f32(a: f32, b: f32) -> f32 { pub extern "C" fn add_f64(a: f64, b: f64) -> f64 { a + b } + +#[no_mangle] +pub extern "C" fn sleep_blocking(ms: u64) { + let duration = Duration::from_millis(ms); + sleep(duration); +} diff --git a/test_ffi/tests/integration_tests.rs b/test_ffi/tests/integration_tests.rs index 62b28d879fafd0..0b2eae854cd9ab 100644 --- a/test_ffi/tests/integration_tests.rs +++ b/test_ffi/tests/integration_tests.rs @@ -46,6 +46,10 @@ fn basic() { 579\n\ 579.9119873046875\n\ 579.912\n\ + Before\n\ + true\n\ + After\n\ + true\n\ Correct number of resources\n"; assert_eq!(stdout, expected); assert_eq!(stderr, ""); diff --git a/test_ffi/tests/test.js b/test_ffi/tests/test.js index 24b64722ca87c9..09c5f74178105d 100644 --- a/test_ffi/tests/test.js +++ b/test_ffi/tests/test.js @@ -20,6 +20,7 @@ const dylib = Deno.dlopen(libPath, { "add_isize": { parameters: ["isize", "isize"], result: "isize" }, "add_f32": { parameters: ["f32", "f32"], result: "f32" }, "add_f64": { parameters: ["f64", "f64"], result: "f64" }, + "sleep_blocking": { parameters: ["u64"], result: "void", nonblocking: true }, }); dylib.symbols.print_something(); @@ -32,16 +33,31 @@ console.log(dylib.symbols.add_isize(123, 456)); console.log(dylib.symbols.add_f32(123.123, 456.789)); console.log(dylib.symbols.add_f64(123.123, 456.789)); -dylib.close(); -const resourcesPost = Deno.resources(); +// Test non blocking calls +const start = performance.now(); +dylib.symbols.sleep_blocking(100).then(() => { + console.log("After"); + console.log(performance.now() - start >= 100); + // Close after task is complete. + cleanup(); +}); +console.log("Before"); +console.log(performance.now() - start < 100); + +function cleanup() { + dylib.close(); -const preStr = JSON.stringify(resourcesPre, null, 2); -const postStr = JSON.stringify(resourcesPost, null, 2); -if (preStr !== postStr) { - throw new Error( - `Difference in open resources before dlopen and after closing: + const resourcesPost = Deno.resources(); + + const preStr = JSON.stringify(resourcesPre, null, 2); + const postStr = JSON.stringify(resourcesPost, null, 2); + if (preStr !== postStr) { + throw new Error( + `Difference in open resources before dlopen and after closing: Before: ${preStr} After: ${postStr}`, - ); + ); + } + + console.log("Correct number of resources"); } -console.log("Correct number of resources"); From 3d0d8246e8f45cb91d9dc9411802091bb6fa2601 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Thu, 30 Sep 2021 06:25:57 +0000 Subject: [PATCH 2/4] fix lint --- ext/ffi/00_ffi.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/ffi/00_ffi.js b/ext/ffi/00_ffi.js index 4e61dc65a21bf0..376105de88889f 100644 --- a/ext/ffi/00_ffi.js +++ b/ext/ffi/00_ffi.js @@ -14,7 +14,7 @@ for (const symbol in symbols) { this.symbols[symbol] = symbols[symbol].nonblocking - ? async (...parameters) => + ? (...parameters) => core.opAsync("op_ffi_call_async", { rid: this.#rid, symbol, From 6528424c401120b374e234a5067a7ffd728849a7 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Fri, 1 Oct 2021 01:06:44 +0000 Subject: [PATCH 3/4] rename op_ffi_call_async => op_ffi_call_nonblocking --- ext/ffi/00_ffi.js | 2 +- ext/ffi/lib.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/ffi/00_ffi.js b/ext/ffi/00_ffi.js index 376105de88889f..c7fdd0e8c58458 100644 --- a/ext/ffi/00_ffi.js +++ b/ext/ffi/00_ffi.js @@ -15,7 +15,7 @@ for (const symbol in symbols) { this.symbols[symbol] = symbols[symbol].nonblocking ? (...parameters) => - core.opAsync("op_ffi_call_async", { + core.opAsync("op_ffi_call_nonblocking", { rid: this.#rid, symbol, parameters, diff --git a/ext/ffi/lib.rs b/ext/ffi/lib.rs index 662b5ec3600fcd..4d78d5e405e579 100644 --- a/ext/ffi/lib.rs +++ b/ext/ffi/lib.rs @@ -113,7 +113,7 @@ pub fn init(unstable: bool) -> Extension { .ops(vec![ ("op_ffi_load", op_sync(op_ffi_load::

)), ("op_ffi_call", op_sync(op_ffi_call)), - ("op_ffi_call_async", op_async(op_ffi_call_async)), + ("op_ffi_call_nonblocking", op_async(op_ffi_call_nonblocking)), ]) .state(move |state| { // Stolen from deno_webgpu, is there a better option? @@ -369,7 +369,7 @@ fn ffi_call(args: FfiCallArgs, symbol: &Symbol) -> Result { }) } -fn op_ffi_call( +fn op_ffi_call_nonblocking( state: &mut deno_core::OpState, args: FfiCallArgs, _: (), From 55ac0a990a7e5b834d157c97f4473a9af026ac3e Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Fri, 1 Oct 2021 01:08:17 +0000 Subject: [PATCH 4/4] fix --- ext/ffi/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/ffi/lib.rs b/ext/ffi/lib.rs index 4d78d5e405e579..2dc7bc2870128f 100644 --- a/ext/ffi/lib.rs +++ b/ext/ffi/lib.rs @@ -369,7 +369,7 @@ fn ffi_call(args: FfiCallArgs, symbol: &Symbol) -> Result { }) } -fn op_ffi_call_nonblocking( +fn op_ffi_call( state: &mut deno_core::OpState, args: FfiCallArgs, _: (), @@ -387,7 +387,7 @@ fn op_ffi_call_nonblocking( } /// A non-blocking FFI call. -async fn op_ffi_call_async( +async fn op_ffi_call_nonblocking( state: Rc>, args: FfiCallArgs, _: (),