summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAapo Alasuutari <aapo.alasuutari@gmail.com>2023-01-29 16:05:08 +0200
committerGitHub <noreply@github.com>2023-01-29 19:35:08 +0530
commit04ba709b6ec994fd084e0d09ff9edc9df3a1eefc (patch)
tree7dc716f982393babcf1011352180866d8b00814e
parent2c7855014498eeae4543aaf6934d3f2bcc818b79 (diff)
perf(ops): Remove unnecessary fast call fallback options usage (#17585)
Currently fast ops will always check for the alignment of a TypedArray when getting a slice out of them. A match is then done to ensure that some slice was received and if not a fallback will be requested. For Uint8Arrays (and WasmMemory which is equivalent to a Uint8Array) the alignment will always be okay. Rust probably optimises this away for the most part (since the Uint8Array check is `x % 1 != 0`), but what it cannot optimise away is the fast ops path's request for fallback options parameter. The extra parameter's cost is likely negligible but V8 will need to check if a fallback was requested and prepare the fallback call just in case it was. In the future the lack of a fallback may also enable V8 to much better optimise the result handling. For V8 created buffers, it seems like all buffers are actually always guaranteed to be properly aligned: All buffers seem to always be created 8-byte aligned, and creating a 32 bit array or 64 bit array with a non-aligned offset from an ArrayBuffer is not allowed. Unfortunately, Deno FFI cannot give the same guarantees, and it is actually possible for eg. 32 bit arrays to be created unaligned using it. These arrays work fine (at least on Linux) so it seems like this is not illegal, it just means that we cannot remove the alignment checking for 32 bit arrays.
-rw-r--r--ext/flash/lib.rs8
-rw-r--r--ops/optimizer.rs29
-rw-r--r--ops/optimizer_tests/op_state_with_transforms.expected2
-rw-r--r--ops/optimizer_tests/op_state_with_transforms.out8
-rw-r--r--ops/optimizer_tests/raw_ptr.out8
-rw-r--r--ops/optimizer_tests/uint8array.expected2
-rw-r--r--ops/optimizer_tests/uint8array.out19
-rw-r--r--ops/optimizer_tests/wasm_op.expected2
8 files changed, 23 insertions, 55 deletions
diff --git a/ext/flash/lib.rs b/ext/flash/lib.rs
index 92490e0b8..273aff02d 100644
--- a/ext/flash/lib.rs
+++ b/ext/flash/lib.rs
@@ -373,11 +373,9 @@ unsafe fn op_flash_respond_fast(
let ctx = &mut *(ptr as *mut ServerContext);
let response = &*response;
- if let Some(response) = response.get_storage_if_aligned() {
- flash_respond(ctx, token, shutdown, response)
- } else {
- todo!();
- }
+ // Uint8Array is always byte-aligned.
+ let response = response.get_storage_if_aligned().unwrap_unchecked();
+ flash_respond(ctx, token, shutdown, response)
}
macro_rules! get_request {
diff --git a/ops/optimizer.rs b/ops/optimizer.rs
index 82593d247..7c19a7cfd 100644
--- a/ops/optimizer.rs
+++ b/ops/optimizer.rs
@@ -126,6 +126,10 @@ impl Transform {
parse_quote! { *const #core::v8::fast_api::FastApiTypedArray<u32> };
q!(Vars { var: &ident }, {
+ // V8 guarantees that ArrayBuffers are always 4-byte aligned
+ // (seems to be always 8-byte aligned on 64-bit machines)
+ // but Deno FFI makes it possible to create ArrayBuffers at any
+ // alignment. Thus this check is needed.
let var = match unsafe { &*var }.get_storage_if_aligned() {
Some(v) => v,
None => {
@@ -141,17 +145,14 @@ impl Transform {
parse_quote! { *const #core::v8::fast_api::FastApiTypedArray<u8> };
q!(Vars { var: &ident }, {
- let var = match unsafe { &*var }.get_storage_if_aligned() {
- Some(v) => v,
- None => {
- unsafe { &mut *fast_api_callback_options }.fallback = true;
- return Default::default();
- }
- };
+ // SAFETY: U8 slice is always byte-aligned.
+ let var =
+ unsafe { (&*var).get_storage_if_aligned().unwrap_unchecked() };
})
}
TransformKind::WasmMemory => {
// Note: `ty` is correctly set to __opts by the fast call tier.
+ // U8 slice is always byte-aligned.
q!(Vars { var: &ident, core }, {
let var = unsafe {
&*(__opts.wasm_memory
@@ -166,13 +167,10 @@ impl Transform {
parse_quote! { *const #core::v8::fast_api::FastApiTypedArray<u8> };
q!(Vars { var: &ident }, {
- let var = match unsafe { &*var }.get_storage_if_aligned() {
- Some(v) => v.as_ptr(),
- None => {
- unsafe { &mut *fast_api_callback_options }.fallback = true;
- return Default::default();
- }
- };
+ // SAFETY: U8 slice is always byte-aligned.
+ let var =
+ unsafe { (&*var).get_storage_if_aligned().unwrap_unchecked() }
+ .as_ptr();
})
}
}
@@ -473,7 +471,6 @@ impl Optimizer {
match segment {
// Is `T` a u8?
PathSegment { ident, .. } if ident == "u8" => {
- self.needs_fast_callback_option = true;
assert!(self
.transforms
.insert(index, Transform::wasm_memory(index))
@@ -608,7 +605,6 @@ impl Optimizer {
match segment {
// Is `T` a u8?
PathSegment { ident, .. } if ident == "u8" => {
- self.needs_fast_callback_option = true;
self.fast_parameters.push(FastValue::Uint8Array);
assert!(self
.transforms
@@ -645,7 +641,6 @@ impl Optimizer {
match segment {
// Is `T` a u8?
PathSegment { ident, .. } if ident == "u8" => {
- self.needs_fast_callback_option = true;
self.fast_parameters.push(FastValue::Uint8Array);
assert!(self
.transforms
diff --git a/ops/optimizer_tests/op_state_with_transforms.expected b/ops/optimizer_tests/op_state_with_transforms.expected
index d6757801e..f5f236b7d 100644
--- a/ops/optimizer_tests/op_state_with_transforms.expected
+++ b/ops/optimizer_tests/op_state_with_transforms.expected
@@ -3,7 +3,7 @@ returns_result: false
has_ref_opstate: true
has_rc_opstate: false
has_fast_callback_option: false
-needs_fast_callback_option: true
+needs_fast_callback_option: false
fast_result: Some(Void)
fast_parameters: [V8Value, Uint8Array]
transforms: {1: Transform { kind: SliceU8(true), index: 1 }}
diff --git a/ops/optimizer_tests/op_state_with_transforms.out b/ops/optimizer_tests/op_state_with_transforms.out
index 541a4fd36..b2ded3c5a 100644
--- a/ops/optimizer_tests/op_state_with_transforms.out
+++ b/ops/optimizer_tests/op_state_with_transforms.out
@@ -142,13 +142,7 @@ where
as *const _ops::OpCtx)
};
let state = &mut ::std::cell::RefCell::borrow_mut(&__ctx.state);
- let buf = match unsafe { &*buf }.get_storage_if_aligned() {
- Some(v) => v,
- None => {
- unsafe { &mut *fast_api_callback_options }.fallback = true;
- return Default::default();
- }
- };
+ let buf = unsafe { (&*buf).get_storage_if_aligned().unwrap_unchecked() };
let result = op_now::call::<TP>(state, buf);
result
}
diff --git a/ops/optimizer_tests/raw_ptr.out b/ops/optimizer_tests/raw_ptr.out
index f9f467ad9..cf678ddb9 100644
--- a/ops/optimizer_tests/raw_ptr.out
+++ b/ops/optimizer_tests/raw_ptr.out
@@ -169,13 +169,7 @@ where
as *const _ops::OpCtx)
};
let state = &mut ::std::cell::RefCell::borrow_mut(&__ctx.state);
- let buf = match unsafe { &*buf }.get_storage_if_aligned() {
- Some(v) => v.as_ptr(),
- None => {
- unsafe { &mut *fast_api_callback_options }.fallback = true;
- return Default::default();
- }
- };
+ let buf = unsafe { (&*buf).get_storage_if_aligned().unwrap_unchecked() }.as_ptr();
let out = match unsafe { &*out }.get_storage_if_aligned() {
Some(v) => v,
None => {
diff --git a/ops/optimizer_tests/uint8array.expected b/ops/optimizer_tests/uint8array.expected
index 0e5667a63..49554a2e8 100644
--- a/ops/optimizer_tests/uint8array.expected
+++ b/ops/optimizer_tests/uint8array.expected
@@ -3,7 +3,7 @@ returns_result: false
has_ref_opstate: false
has_rc_opstate: false
has_fast_callback_option: false
-needs_fast_callback_option: true
+needs_fast_callback_option: false
fast_result: Some(Bool)
fast_parameters: [V8Value, Uint8Array, Uint8Array]
transforms: {0: Transform { kind: SliceU8(false), index: 0 }, 1: Transform { kind: SliceU8(true), index: 1 }}
diff --git a/ops/optimizer_tests/uint8array.out b/ops/optimizer_tests/uint8array.out
index 90d420fa0..bd7fa4d87 100644
--- a/ops/optimizer_tests/uint8array.out
+++ b/ops/optimizer_tests/uint8array.out
@@ -158,7 +158,7 @@ impl<'scope> deno_core::v8::fast_api::FastFunction for op_import_spki_x25519_fas
fn args(&self) -> &'static [deno_core::v8::fast_api::Type] {
use deno_core::v8::fast_api::Type::*;
use deno_core::v8::fast_api::CType;
- &[V8Value, TypedArray(CType::Uint8), TypedArray(CType::Uint8), CallbackOptions]
+ &[V8Value, TypedArray(CType::Uint8), TypedArray(CType::Uint8)]
}
fn return_type(&self) -> deno_core::v8::fast_api::CType {
deno_core::v8::fast_api::CType::Bool
@@ -168,24 +168,11 @@ fn op_import_spki_x25519_fast_fn<'scope>(
_: deno_core::v8::Local<deno_core::v8::Object>,
key_data: *const deno_core::v8::fast_api::FastApiTypedArray<u8>,
out: *const deno_core::v8::fast_api::FastApiTypedArray<u8>,
- fast_api_callback_options: *mut deno_core::v8::fast_api::FastApiCallbackOptions,
) -> bool {
use deno_core::v8;
use deno_core::_ops;
- let key_data = match unsafe { &*key_data }.get_storage_if_aligned() {
- Some(v) => v,
- None => {
- unsafe { &mut *fast_api_callback_options }.fallback = true;
- return Default::default();
- }
- };
- let out = match unsafe { &*out }.get_storage_if_aligned() {
- Some(v) => v,
- None => {
- unsafe { &mut *fast_api_callback_options }.fallback = true;
- return Default::default();
- }
- };
+ let key_data = unsafe { (&*key_data).get_storage_if_aligned().unwrap_unchecked() };
+ let out = unsafe { (&*out).get_storage_if_aligned().unwrap_unchecked() };
let result = op_import_spki_x25519::call(key_data, out);
result
}
diff --git a/ops/optimizer_tests/wasm_op.expected b/ops/optimizer_tests/wasm_op.expected
index 98cfb4e7d..8d3719cf7 100644
--- a/ops/optimizer_tests/wasm_op.expected
+++ b/ops/optimizer_tests/wasm_op.expected
@@ -3,7 +3,7 @@ returns_result: false
has_ref_opstate: false
has_rc_opstate: false
has_fast_callback_option: false
-needs_fast_callback_option: true
+needs_fast_callback_option: false
fast_result: Some(Void)
fast_parameters: [V8Value]
transforms: {0: Transform { kind: WasmMemory, index: 0 }}