Repository: denoland/deno_core
Pull request #255 (2023-10-03 19:27:36)

feat(ops): support bigint fastcall returns
Benchmarks - bigint return values are pretty fast (nearly the same as an external).

```
test baseline                               ... bench:         498 ns/iter (+/- 6)
test bench_op_arraybuffer                   ... bench:       4,288 ns/iter (+/- 47)
test bench_op_bigint                        ... bench:      42,919 ns/iter (+/- 5,518)
test bench_op_bigint_return                 ... bench:       2,327 ns/iter (+/- 35)
test bench_op_buffer                        ... bench:       3,126 ns/iter (+/- 57)
test bench_op_buffer_nofast                 ... bench:      31,840 ns/iter (+/- 1,142)
test bench_op_external                      ... bench:       2,444 ns/iter (+/- 43)
test bench_op_external_nofast               ... bench:       8,531 ns/iter (+/- 106)
test bench_op_option_u32                    ... bench:       6,073 ns/iter (+/- 227)
test bench_op_string_option_u32             ... bench:      26,398 ns/iter (+/- 838)
test bench_op_u32                           ... bench:       2,264 ns/iter (+/- 41)
```




Patch:
diff --git a/core/benches/ops/sync.rs b/core/benches/ops/sync.rs
index fa8d8de..8231e68 100644
--- a/core/benches/ops/sync.rs
+++ b/core/benches/ops/sync.rs
@@ -22,6 +22,8 @@ deno_core::extension!(
     op_global,
     op_global_scope,
     op_make_external,
+    op_bigint,
+    op_bigint_return,
     op_external,
     op_external_nofast,
     op_buffer,
@@ -97,6 +99,15 @@ pub fn op_make_external() -> *const c_void {
   std::ptr::null()
 }
 
+#[op2(fast)]
+pub fn op_bigint(#[bigint] _input: u64) {}
+
+#[op2(fast)]
+#[bigint]
+pub fn op_bigint_return() -> u64 {
+  0
+}
+
 #[op2(fast)]
 pub fn op_external(_input: *const c_void) {}
 
@@ -380,6 +391,20 @@ fn bench_op_v8_global_scope(b: &mut Bencher) {
   bench_op(b, BENCH_COUNT, "op_global_scope", 1, "op_global_scope('this is a reasonably long string that we would like to get the length of!');");
 }
 
+fn bench_op_bigint(b: &mut Bencher) {
+  bench_op(b, BENCH_COUNT, "op_bigint", 1, "op_bigint(0n);");
+}
+
+fn bench_op_bigint_return(b: &mut Bencher) {
+  bench_op(
+    b,
+    BENCH_COUNT,
+    "op_bigint_return",
+    1,
+    "accum += op_bigint_return();",
+  );
+}
+
 fn bench_op_external(b: &mut Bencher) {
   bench_op(b, BENCH_COUNT, "op_external", 1, "op_external(EXTERNAL)");
 }
@@ -444,6 +469,8 @@ benchmark_group!(
   bench_op_v8_local_nofast,
   bench_op_v8_global,
   bench_op_v8_global_scope,
+  bench_op_bigint,
+  bench_op_bigint_return,
   bench_op_external,
   bench_op_external_nofast,
   bench_op_buffer,
diff --git a/core/ops.rs b/core/ops.rs
index df30507..3cd8fbc 100644
--- a/core/ops.rs
+++ b/core/ops.rs
@@ -24,7 +24,6 @@ use std::rc::Weak;
 use std::sync::Arc;
 use v8::fast_api::CFunctionInfo;
 use v8::fast_api::CTypeInfo;
-use v8::fast_api::Int64Representation;
 use v8::Isolate;
 
 pub type PromiseId = i32;
@@ -194,9 +193,7 @@ impl OpCtx {
           args.as_ptr(),
           fast_fn.args.len(),
           ret.as_ptr(),
-          // TODO(bartlomieju): in the future we might want to change it
-          // to use BigInt representation.
-          Int64Representation::Number,
+          fast_fn.repr,
         )
       };
       fast_fn_c_info = Some(c_fn);
diff --git a/core/runtime/ops.rs b/core/runtime/ops.rs
index 70d5736..6f178e5 100644
--- a/core/runtime/ops.rs
+++ b/core/runtime/ops.rs
@@ -1198,13 +1198,13 @@ mod tests {
     Ok(())
   }
 
-  #[op2(core)]
+  #[op2(core, fast)]
   #[bigint]
   pub fn op_test_bigint_u64(#[bigint] input: u64) -> u64 {
     input
   }
 
-  #[op2(core)]
+  #[op2(core, fast)]
   #[bigint]
   pub fn op_test_bigint_i64(#[bigint] input: i64) -> i64 {
     input
diff --git a/ops/op2/README.md b/ops/op2/README.md
index f0fb6fd..0d0dd91 100644
--- a/ops/op2/README.md
+++ b/ops/op2/README.md
@@ -1153,7 +1153,7 @@ SMI is internally represented as a signed integer, but unsigned `#[smi]` types w
 ```
 
 </td><td>
-
+✅
 </td><td>
 
 </td><td>
@@ -1169,7 +1169,7 @@ SMI is internally represented as a signed integer, but unsigned `#[smi]` types w
 ```
 
 </td><td>
-
+✅
 </td><td>
 
 </td><td>
@@ -1185,7 +1185,7 @@ SMI is internally represented as a signed integer, but unsigned `#[smi]` types w
 ```
 
 </td><td>
-
+✅
 </td><td>
 
 </td><td>
@@ -1201,7 +1201,7 @@ SMI is internally represented as a signed integer, but unsigned `#[smi]` types w
 ```
 
 </td><td>
-
+✅
 </td><td>
 
 </td><td>
diff --git a/ops/op2/dispatch_fast.rs b/ops/op2/dispatch_fast.rs
index 1bb8089..84ad64d 100644
--- a/ops/op2/dispatch_fast.rs
+++ b/ops/op2/dispatch_fast.rs
@@ -302,8 +302,6 @@ pub fn generate_dispatch_fast(
   let fast_definition = gs_quote!(generator_state(deno_core) => {
     use #deno_core::v8::fast_api::Type;
     use #deno_core::v8::fast_api::CType;
-    // TODO(mmastrac): We're setting this up for future success but returning
-    // u64/i64 from fastcall functions does not work. Test again in the future.
     #deno_core::v8::fast_api::FastFunction::new_with_bigint(
       &[ Type::V8Value, #( #input_types ),* ],
       #output_type,
@@ -314,8 +312,6 @@ pub fn generate_dispatch_fast(
   let fast_definition_metrics = gs_quote!(generator_state(deno_core) => {
     use #deno_core::v8::fast_api::Type;
     use #deno_core::v8::fast_api::CType;
-    // TODO(mmastrac): We're setting this up for future success but returning
-    // u64/i64 from fastcall functions does not work. Test again in the future.
     #deno_core::v8::fast_api::FastFunction::new_with_bigint(
       &[ Type::V8Value, #( #input_types_metrics ),* ],
       #output_type,
@@ -644,14 +640,10 @@ fn map_retval_to_v8_fastcall_type(
     | Arg::Numeric(NumericArg::i16, _)
     | Arg::Numeric(NumericArg::i8, _) => V8FastCallType::I32,
     Arg::Numeric(NumericArg::u64 | NumericArg::usize, NumericFlag::None) => {
-      // TODO(mmastrac): In my testing, I was not able to get this working properly
-      // V8FastCallType::U64
-      return Ok(None);
+      V8FastCallType::U64
     }
     Arg::Numeric(NumericArg::i64 | NumericArg::isize, NumericFlag::None) => {
-      // TODO(mmastrac): In my testing, I was not able to get this working properly
-      // V8FastCallType::I64
-      return Ok(None);
+      V8FastCallType::I64
     }
     Arg::Numeric(
       NumericArg::u64 | NumericArg::usize | NumericArg::i64 | NumericArg::isize,
diff --git a/ops/op2/valid_retvals.md b/ops/op2/valid_retvals.md
index 1f87dad..c9a8cce 100644
--- a/ops/op2/valid_retvals.md
+++ b/ops/op2/valid_retvals.md
@@ -8,10 +8,10 @@
 | X         | i32                              | X        | Int32       |                                                                                                                                                                                                  |
 | X         | u32                              | X        | Uint32      |                                                                                                                                                                                                  |
 | X         | #[smi] ResourceId                | X        | Int32       | SMI is internally represented as a signed integer, but unsigned `#[smi]` types will be bit-converted to unsigned values for the Rust call. JavaScript code will continue to see signed integers. |
-| X         | #[bigint] i64                    |          | BigInt      |                                                                                                                                                                                                  |
-| X         | #[bigint] u64                    |          | BigInt      |                                                                                                                                                                                                  |
-| X         | #[bigint] isize                  |          | BigInt      |                                                                                                                                                                                                  |
-| X         | #[bigint] usize                  |          | BigInt      |                                                                                                                                                                                                  |
+| X         | #[bigint] i64                    | X        | BigInt      |                                                                                                                                                                                                  |
+| X         | #[bigint] u64                    | X        | BigInt      |                                                                                                                                                                                                  |
+| X         | #[bigint] isize                  | X        | BigInt      |                                                                                                                                                                                                  |
+| X         | #[bigint] usize                  | X        | BigInt      |                                                                                                                                                                                                  |
 | X         | #[number] i64                    | X        | Number      | Result must fit within `Number.MIN_SAFE_INTEGER` and `Number.MAX_SAFE_INTEGER`                                                                                                                   |
 | X         | #[number] u64                    | X        | Number      | Result must fit within `Number.MIN_SAFE_INTEGER` and `Number.MAX_SAFE_INTEGER`                                                                                                                   |
 | X         | #[number] isize                  | X        | Number      | Result must fit within `Number.MIN_SAFE_INTEGER` and `Number.MAX_SAFE_INTEGER`                                                                                                                   |


Test patch:
diff --git a/ops/op2/test_cases/sync/bigint.out b/ops/op2/test_cases/sync/bigint.out
new file mode 100644
index 0000000..291157f
--- /dev/null
+++ b/ops/op2/test_cases/sync/bigint.out
@@ -0,0 +1,99 @@
+#[allow(non_camel_case_types)]
+pub struct op_bigint {
+    _unconstructable: ::std::marker::PhantomData<()>,
+}
+impl deno_core::_ops::Op for op_bigint {
+    const NAME: &'static str = stringify!(op_bigint);
+    const DECL: deno_core::_ops::OpDecl = deno_core::_ops::OpDecl::new_internal_op2(
+        stringify!(op_bigint),
+        false,
+        0usize as u8,
+        Self::v8_fn_ptr as _,
+        Self::v8_fn_ptr_metrics as _,
+        Some({
+            use deno_core::v8::fast_api::Type;
+            use deno_core::v8::fast_api::CType;
+            deno_core::v8::fast_api::FastFunction::new_with_bigint(
+                &[Type::V8Value],
+                CType::Uint64,
+                Self::v8_fn_ptr_fast as *const ::std::ffi::c_void,
+            )
+        }),
+        Some({
+            use deno_core::v8::fast_api::Type;
+            use deno_core::v8::fast_api::CType;
+            deno_core::v8::fast_api::FastFunction::new_with_bigint(
+                &[Type::V8Value, Type::CallbackOptions],
+                CType::Uint64,
+                Self::v8_fn_ptr_fast_metrics as *const ::std::ffi::c_void,
+            )
+        }),
+    );
+}
+impl op_bigint {
+    pub const fn name() -> &'static str {
+        stringify!(op_bigint)
+    }
+    #[deprecated(note = "Use the const op::DECL instead")]
+    pub const fn decl() -> deno_core::_ops::OpDecl {
+        <Self as deno_core::_ops::Op>::DECL
+    }
+    #[allow(clippy::too_many_arguments)]
+    fn v8_fn_ptr_fast_metrics(
+        this: deno_core::v8::Local<deno_core::v8::Object>,
+        fast_api_callback_options: *mut deno_core::v8::fast_api::FastApiCallbackOptions,
+    ) -> u64 {
+        let fast_api_callback_options = unsafe { &mut *fast_api_callback_options };
+        let opctx = unsafe {
+            &*(deno_core::v8::Local::<
+                deno_core::v8::External,
+            >::cast(unsafe { fast_api_callback_options.data.data })
+                .value() as *const deno_core::_ops::OpCtx)
+        };
+        deno_core::_ops::dispatch_metrics_fast(
+            &opctx,
+            deno_core::_ops::OpMetricsEvent::Dispatched,
+        );
+        let res = Self::v8_fn_ptr_fast(this);
+        deno_core::_ops::dispatch_metrics_fast(
+            &opctx,
+            deno_core::_ops::OpMetricsEvent::Completed,
+        );
+        res
+    }
+    #[allow(clippy::too_many_arguments)]
+    fn v8_fn_ptr_fast(_: deno_core::v8::Local<deno_core::v8::Object>) -> u64 {
+        let result = { Self::call() };
+        result as _
+    }
+    extern "C" fn v8_fn_ptr(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let mut scope = unsafe { deno_core::v8::CallbackScope::new(&*info) };
+        let mut rv = deno_core::v8::ReturnValue::from_function_callback_info(unsafe {
+            &*info
+        });
+        let result = { Self::call() };
+        rv.set(deno_core::_ops::RustToV8::to_v8(result, &mut scope))
+    }
+    extern "C" fn v8_fn_ptr_metrics(info: *const deno_core::v8::FunctionCallbackInfo) {
+        let args = deno_core::v8::FunctionCallbackArguments::from_function_callback_info(unsafe {
+            &*info
+        });
+        let opctx = unsafe {
+            &*(deno_core::v8::Local::<deno_core::v8::External>::cast(args.data()).value()
+                as *const deno_core::_ops::OpCtx)
+        };
+        deno_core::_ops::dispatch_metrics_slow(
+            &opctx,
+            deno_core::_ops::OpMetricsEvent::Dispatched,
+        );
+        Self::v8_fn_ptr(info);
+        deno_core::_ops::dispatch_metrics_slow(
+            &opctx,
+            deno_core::_ops::OpMetricsEvent::Completed,
+        );
+    }
+    #[inline(always)]
+    pub fn call() -> u64 {
+        0
+    }
+}
diff --git a/ops/op2/test_cases/sync/bigint.rs b/ops/op2/test_cases/sync/bigint.rs
new file mode 100644
index 0000000..e4ea937
--- /dev/null
+++ b/ops/op2/test_cases/sync/bigint.rs
@@ -0,0 +1,9 @@
+// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+#![deny(warnings)]
+deno_ops_compile_test_runner::prelude!();
+
+#[op2(fast)]
+#[bigint]
+pub fn op_bigint() -> u64 {
+  0
+}