Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(web): optimize encodeInto() #15922

Merged
merged 10 commits into from
Sep 17, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 23 additions & 0 deletions cli/bench/encode_into.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2018-2022 the Deno authors. All rights reserved. MIT license.
const queueMicrotask = globalThis.queueMicrotask || process.nextTick;
let [total, count] = typeof Deno !== "undefined"
? Deno.args
: [process.argv[2], process.argv[3]];

total = total ? parseInt(total, 0) : 50;
count = count ? parseInt(count, 10) : 1000000;

function bench(fun) {
const start = Date.now();
for (let i = 0; i < count; i++) fun();
const elapsed = Date.now() - start;
const rate = Math.floor(count / (elapsed / 1000));
console.log(`time ${elapsed} ms rate ${rate}`);
if (--total) queueMicrotask(() => bench(fun));
}

const encoder = new TextEncoder();
const data = "hello world";
const out = new Uint8Array(100);

bench(() => encoder.encodeInto(data, out));
2 changes: 1 addition & 1 deletion core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ serde_json = { version = "1.0.79", features = ["preserve_order"] }
serde_v8 = { version = "0.62.0", path = "../serde_v8" }
sourcemap = "=6.0.1"
url = { version = "2.3.1", features = ["serde", "expose_internals"] }
v8 = { version = "0.49.0", default-features = false }
v8 = { version = "0.50.0", default-features = false }

[[example]]
name = "http_bench_json_ops"
Expand Down
2 changes: 1 addition & 1 deletion core/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,7 @@ impl JsRuntime {
{
let scope = &mut self.handle_scope();
let o = Self::grab_global::<v8::Object>(scope, "Deno.core.ops").unwrap();
let names = o.get_own_property_names(scope).unwrap();
let names = o.get_own_property_names(scope, Default::default()).unwrap();
for i in 0..names.length() {
let key = names.get_index(scope, i).unwrap();
o.delete(scope, key);
Expand Down
9 changes: 8 additions & 1 deletion ext/web/08_text_encoding.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
TypedArrayPrototypeSubarray,
TypedArrayPrototypeSlice,
Uint8Array,
Uint32Array,
} = window.__bootstrap.primordials;

class TextDecoder {
Expand Down Expand Up @@ -199,10 +200,16 @@
context: "Argument 2",
allowShared: true,
});
return ops.op_encoding_encode_into(source, destination);
ops.op_encoding_encode_into(source, destination, encodeIntoBuf);
return {
read: encodeIntoBuf[0],
written: encodeIntoBuf[1],
};
}
}

const encodeIntoBuf = new Uint32Array(2);

webidl.configurePrototype(TextEncoder);
const TextEncoderPrototype = TextEncoder.prototype;

Expand Down
59 changes: 22 additions & 37 deletions ext/web/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ use deno_core::error::type_error;
use deno_core::error::AnyError;
use deno_core::include_js_files;
use deno_core::op;
use deno_core::serde_v8;
use deno_core::url::Url;
use deno_core::v8;
use deno_core::ByteString;
use deno_core::CancelHandle;
use deno_core::Extension;
Expand All @@ -19,11 +21,11 @@ use deno_core::Resource;
use deno_core::ResourceId;
use deno_core::U16String;
use deno_core::ZeroCopyBuf;

use encoding_rs::CoderResult;
use encoding_rs::Decoder;
use encoding_rs::DecoderResult;
use encoding_rs::Encoding;
use serde::Serialize;
use std::borrow::Cow;
use std::cell::RefCell;
use std::fmt;
Expand Down Expand Up @@ -314,46 +316,29 @@ impl Resource for TextDecoderResource {
}
}

#[derive(Serialize)]
#[serde(rename_all = "camelCase")]
struct EncodeIntoResult {
read: usize,
written: usize,
}
littledivy marked this conversation as resolved.
Show resolved Hide resolved

#[op]
#[op(v8)]
fn op_encoding_encode_into(
input: String,
scope: &mut v8::HandleScope,
input: serde_v8::Value,
buffer: &mut [u8],
) -> EncodeIntoResult {
// Since `input` is already UTF-8, we can simply find the last UTF-8 code
// point boundary from input that fits in `buffer`, and copy the bytes up to
// that point.
let boundary = if buffer.len() >= input.len() {
input.len()
} else {
let mut boundary = buffer.len();

// The maximum length of a UTF-8 code point is 4 bytes.
for _ in 0..4 {
if input.is_char_boundary(boundary) {
break;
}
debug_assert!(boundary > 0);
boundary -= 1;
}

debug_assert!(input.is_char_boundary(boundary));
boundary
out_buf: &mut [u8],
) {
let s = v8::Local::<v8::String>::try_from(input.v8_value).unwrap();
littledivy marked this conversation as resolved.
Show resolved Hide resolved
assert!(out_buf.len() == std::mem::size_of::<u32>() * 2);
// SAFETY: `out_buf` is guaranteed to be large enough to hold result.
let out_buf: &mut [u32] = unsafe {
std::slice::from_raw_parts_mut(out_buf.as_mut_ptr() as *mut u32, 2)
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't understand what guarantees alignment. Can you explain?

Note that getting the alignment wrong can lead to miscompiles, even on x86_64.

Copy link

@ghost ghost Sep 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In JS, ArrayBuffers with a length ≥ the necessary alignment of type T, are usable as that type.

Upon second thought though, as this accepts TypedArrays and turns them into &mut [u8], then it's still unsound, as one may pass new Uint8Array(5).subarray(1), which has a byteOffset that isn't aligned to a u32 boundary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yikes, lemme add &mut [u32] support to #[ops] to directly deal with Uint32Array


buffer[..boundary].copy_from_slice(input[..boundary].as_bytes());

EncodeIntoResult {
// The `read` output parameter is measured in UTF-16 code units.
read: input[..boundary].encode_utf16().count(),
written: boundary,
}
let mut nchars = 0;
out_buf[1] = s.write_utf8(
scope,
buffer,
Some(&mut nchars),
v8::WriteOptions::NO_NULL_TERMINATION
| v8::WriteOptions::REPLACE_INVALID_UTF8,
) as u32;
out_buf[0] = nchars as u32;
}

/// Creates a [`CancelHandle`] resource that can be used to cancel invocations of certain ops.
Expand Down
47 changes: 19 additions & 28 deletions ops/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -721,39 +721,30 @@ fn codegen_arg(
};
}
// Fast path for &/&mut [u8] and &/&mut [u32]
if let Some(ty) = is_ref_slice(&**ty) {
let (ptr_ty, mutability) = match ty {
SliceType::U8 => (quote!([u8]), quote!(&)),
SliceType::U8Mut => (quote!([u8]), quote!(&mut)),
};
if is_ref_slice(&**ty).is_some() {
return quote! {
let #ident = {
let value = args.get(#idx as i32);
if let Ok(view) = #core::v8::Local::<#core::v8::ArrayBufferView>::try_from(value) {
let (offset, len) = (view.byte_offset(), view.byte_length());
let buffer = match view.buffer(scope) {
Some(v) => v,
None => {
return #core::_ops::throw_type_error(scope, format!("Expected ArrayBufferView at position {}", #idx));
}
};
let store = buffer.get_backing_store();
if store.is_shared() {
return #core::_ops::throw_type_error(scope, format!("Expected non-shared ArrayBufferView at position {}", #idx));
}
unsafe { #mutability *(&store[offset..offset + len] as *const _ as *mut #ptr_ty) }
} else {
let b: #core::v8::Local<#core::v8::ArrayBuffer> = match value.try_into() {
Ok(v) => v,
Err(_) => {
return #core::_ops::throw_type_error(scope, format!("Expected ArrayBuffer at position {}", #idx));
match #core::v8::Local::<#core::v8::ArrayBuffer>::try_from(value) {
Ok(b) => {
let store = b.data() as *mut u8;
unsafe { ::std::slice::from_raw_parts_mut(store, b.byte_length()) }
},
Err(_) => {
if let Ok(view) = #core::v8::Local::<#core::v8::ArrayBufferView>::try_from(value) {
let (offset, len) = (view.byte_offset(), view.byte_length());
let buffer = match view.buffer(scope) {
Some(v) => v,
None => {
return #core::_ops::throw_type_error(scope, format!("Expected ArrayBufferView at position {}", #idx));
}
};
let store = buffer.data() as *mut u8;
unsafe { ::std::slice::from_raw_parts_mut(store.add(offset), len) }
} else {
return #core::_ops::throw_type_error(scope, format!("Expected ArrayBufferView at position {}", #idx));
}
};
let store = b.get_backing_store();
if store.is_shared() {
return #core::_ops::throw_type_error(scope, format!("Expected non-shared ArrayBufferView at position {}", #idx));
}
unsafe { #mutability *(&store[0..b.byte_length()] as *const _ as *mut #ptr_ty) }
}
};
};
Expand Down
2 changes: 1 addition & 1 deletion serde_v8/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ derive_more = "0.99.17"
serde = { version = "1.0.136", features = ["derive"] }
serde_bytes = "0.11"
smallvec = { version = "1.8", features = ["union"] }
v8 = { version = "0.49.0", default-features = false }
v8 = { version = "0.50.0", default-features = false }

[dev-dependencies]
bencher = "0.1"
Expand Down
6 changes: 4 additions & 2 deletions serde_v8/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,8 @@ impl<'de, 'a, 'b, 's, 'x> de::Deserializer<'de>
};
visitor.visit_map(map)
} else {
let prop_names = obj.get_own_property_names(self.scope);
let prop_names =
obj.get_own_property_names(self.scope, Default::default());
let keys: Vec<magic::Value> = match prop_names {
Some(names) => from_v8(self.scope, names.into()).unwrap(),
None => vec![],
Expand Down Expand Up @@ -410,7 +411,8 @@ impl<'de, 'a, 'b, 's, 'x> de::Deserializer<'de>
let obj = v8::Local::<v8::Object>::try_from(self.input).unwrap();
// Unpack single-key
let tag = {
let prop_names = obj.get_own_property_names(self.scope);
let prop_names =
obj.get_own_property_names(self.scope, Default::default());
let prop_names = prop_names.ok_or(Error::ExpectedEnum)?;
if prop_names.length() != 1 {
return Err(Error::LengthMismatch);
Expand Down
97 changes: 7 additions & 90 deletions tools/wpt/expectation.json
Original file line number Diff line number Diff line change
Expand Up @@ -991,94 +991,8 @@
"api-replacement-encodings.any.worker.html": true,
"api-surrogates-utf8.any.html": true,
"api-surrogates-utf8.any.worker.html": true,
"encodeInto.any.html": [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

"encodeInto() into SharedArrayBuffer with Hi and destination length 0, offset 0, filler 0",
"encodeInto() into SharedArrayBuffer with Hi and destination length 0, offset 4, filler 0",
"encodeInto() into SharedArrayBuffer with Hi and destination length 0, offset 0, filler 128",
"encodeInto() into SharedArrayBuffer with Hi and destination length 0, offset 4, filler 128",
"encodeInto() into SharedArrayBuffer with Hi and destination length 0, offset 0, filler random",
"encodeInto() into SharedArrayBuffer with Hi and destination length 0, offset 4, filler random",
"encodeInto() into SharedArrayBuffer with A and destination length 10, offset 0, filler 0",
"encodeInto() into SharedArrayBuffer with A and destination length 10, offset 4, filler 0",
"encodeInto() into SharedArrayBuffer with A and destination length 10, offset 0, filler 128",
"encodeInto() into SharedArrayBuffer with A and destination length 10, offset 4, filler 128",
"encodeInto() into SharedArrayBuffer with A and destination length 10, offset 0, filler random",
"encodeInto() into SharedArrayBuffer with A and destination length 10, offset 4, filler random",
"encodeInto() into SharedArrayBuffer with 𝌆 and destination length 4, offset 0, filler 0",
"encodeInto() into SharedArrayBuffer with 𝌆 and destination length 4, offset 4, filler 0",
"encodeInto() into SharedArrayBuffer with 𝌆 and destination length 4, offset 0, filler 128",
"encodeInto() into SharedArrayBuffer with 𝌆 and destination length 4, offset 4, filler 128",
"encodeInto() into SharedArrayBuffer with 𝌆 and destination length 4, offset 0, filler random",
"encodeInto() into SharedArrayBuffer with 𝌆 and destination length 4, offset 4, filler random",
"encodeInto() into SharedArrayBuffer with 𝌆A and destination length 3, offset 0, filler 0",
"encodeInto() into SharedArrayBuffer with 𝌆A and destination length 3, offset 4, filler 0",
"encodeInto() into SharedArrayBuffer with 𝌆A and destination length 3, offset 0, filler 128",
"encodeInto() into SharedArrayBuffer with 𝌆A and destination length 3, offset 4, filler 128",
"encodeInto() into SharedArrayBuffer with 𝌆A and destination length 3, offset 0, filler random",
"encodeInto() into SharedArrayBuffer with 𝌆A and destination length 3, offset 4, filler random",
"encodeInto() into SharedArrayBuffer with \ud834A\udf06A¥Hi and destination length 10, offset 0, filler 0",
"encodeInto() into SharedArrayBuffer with \ud834A\udf06A¥Hi and destination length 10, offset 4, filler 0",
"encodeInto() into SharedArrayBuffer with \ud834A\udf06A¥Hi and destination length 10, offset 0, filler 128",
"encodeInto() into SharedArrayBuffer with \ud834A\udf06A¥Hi and destination length 10, offset 4, filler 128",
"encodeInto() into SharedArrayBuffer with \ud834A\udf06A¥Hi and destination length 10, offset 0, filler random",
"encodeInto() into SharedArrayBuffer with \ud834A\udf06A¥Hi and destination length 10, offset 4, filler random",
"encodeInto() into SharedArrayBuffer with A\udf06 and destination length 4, offset 0, filler 0",
"encodeInto() into SharedArrayBuffer with A\udf06 and destination length 4, offset 4, filler 0",
"encodeInto() into SharedArrayBuffer with A\udf06 and destination length 4, offset 0, filler 128",
"encodeInto() into SharedArrayBuffer with A\udf06 and destination length 4, offset 4, filler 128",
"encodeInto() into SharedArrayBuffer with A\udf06 and destination length 4, offset 0, filler random",
"encodeInto() into SharedArrayBuffer with A\udf06 and destination length 4, offset 4, filler random",
"encodeInto() into SharedArrayBuffer with ¥¥ and destination length 4, offset 0, filler 0",
"encodeInto() into SharedArrayBuffer with ¥¥ and destination length 4, offset 4, filler 0",
"encodeInto() into SharedArrayBuffer with ¥¥ and destination length 4, offset 0, filler 128",
"encodeInto() into SharedArrayBuffer with ¥¥ and destination length 4, offset 4, filler 128",
"encodeInto() into SharedArrayBuffer with ¥¥ and destination length 4, offset 0, filler random",
"encodeInto() into SharedArrayBuffer with ¥¥ and destination length 4, offset 4, filler random"
],
"encodeInto.any.worker.html": [
"encodeInto() into SharedArrayBuffer with Hi and destination length 0, offset 0, filler 0",
"encodeInto() into SharedArrayBuffer with Hi and destination length 0, offset 4, filler 0",
"encodeInto() into SharedArrayBuffer with Hi and destination length 0, offset 0, filler 128",
"encodeInto() into SharedArrayBuffer with Hi and destination length 0, offset 4, filler 128",
"encodeInto() into SharedArrayBuffer with Hi and destination length 0, offset 0, filler random",
"encodeInto() into SharedArrayBuffer with Hi and destination length 0, offset 4, filler random",
"encodeInto() into SharedArrayBuffer with A and destination length 10, offset 0, filler 0",
"encodeInto() into SharedArrayBuffer with A and destination length 10, offset 4, filler 0",
"encodeInto() into SharedArrayBuffer with A and destination length 10, offset 0, filler 128",
"encodeInto() into SharedArrayBuffer with A and destination length 10, offset 4, filler 128",
"encodeInto() into SharedArrayBuffer with A and destination length 10, offset 0, filler random",
"encodeInto() into SharedArrayBuffer with A and destination length 10, offset 4, filler random",
"encodeInto() into SharedArrayBuffer with 𝌆 and destination length 4, offset 0, filler 0",
"encodeInto() into SharedArrayBuffer with 𝌆 and destination length 4, offset 4, filler 0",
"encodeInto() into SharedArrayBuffer with 𝌆 and destination length 4, offset 0, filler 128",
"encodeInto() into SharedArrayBuffer with 𝌆 and destination length 4, offset 4, filler 128",
"encodeInto() into SharedArrayBuffer with 𝌆 and destination length 4, offset 0, filler random",
"encodeInto() into SharedArrayBuffer with 𝌆 and destination length 4, offset 4, filler random",
"encodeInto() into SharedArrayBuffer with 𝌆A and destination length 3, offset 0, filler 0",
"encodeInto() into SharedArrayBuffer with 𝌆A and destination length 3, offset 4, filler 0",
"encodeInto() into SharedArrayBuffer with 𝌆A and destination length 3, offset 0, filler 128",
"encodeInto() into SharedArrayBuffer with 𝌆A and destination length 3, offset 4, filler 128",
"encodeInto() into SharedArrayBuffer with 𝌆A and destination length 3, offset 0, filler random",
"encodeInto() into SharedArrayBuffer with 𝌆A and destination length 3, offset 4, filler random",
"encodeInto() into SharedArrayBuffer with \ud834A\udf06A¥Hi and destination length 10, offset 0, filler 0",
"encodeInto() into SharedArrayBuffer with \ud834A\udf06A¥Hi and destination length 10, offset 4, filler 0",
"encodeInto() into SharedArrayBuffer with \ud834A\udf06A¥Hi and destination length 10, offset 0, filler 128",
"encodeInto() into SharedArrayBuffer with \ud834A\udf06A¥Hi and destination length 10, offset 4, filler 128",
"encodeInto() into SharedArrayBuffer with \ud834A\udf06A¥Hi and destination length 10, offset 0, filler random",
"encodeInto() into SharedArrayBuffer with \ud834A\udf06A¥Hi and destination length 10, offset 4, filler random",
"encodeInto() into SharedArrayBuffer with A\udf06 and destination length 4, offset 0, filler 0",
"encodeInto() into SharedArrayBuffer with A\udf06 and destination length 4, offset 4, filler 0",
"encodeInto() into SharedArrayBuffer with A\udf06 and destination length 4, offset 0, filler 128",
"encodeInto() into SharedArrayBuffer with A\udf06 and destination length 4, offset 4, filler 128",
"encodeInto() into SharedArrayBuffer with A\udf06 and destination length 4, offset 0, filler random",
"encodeInto() into SharedArrayBuffer with A\udf06 and destination length 4, offset 4, filler random",
"encodeInto() into SharedArrayBuffer with ¥¥ and destination length 4, offset 0, filler 0",
"encodeInto() into SharedArrayBuffer with ¥¥ and destination length 4, offset 4, filler 0",
"encodeInto() into SharedArrayBuffer with ¥¥ and destination length 4, offset 0, filler 128",
"encodeInto() into SharedArrayBuffer with ¥¥ and destination length 4, offset 4, filler 128",
"encodeInto() into SharedArrayBuffer with ¥¥ and destination length 4, offset 0, filler random",
"encodeInto() into SharedArrayBuffer with ¥¥ and destination length 4, offset 4, filler random"
],
"encodeInto.any.html": true,
"encodeInto.any.worker.html": true,
"idlharness.any.html": true,
"idlharness.any.worker.html": true,
"iso-2022-jp-decoder.any.html": true,
Expand Down Expand Up @@ -1117,7 +1031,8 @@
"encode-utf8.any.html": true,
"encode-utf8.any.worker.html": true,
"readable-writable-properties.any.html": true,
"readable-writable-properties.any.worker.html": true
"readable-writable-properties.any.worker.html": true,
"realms.window.html": false
},
"textdecoder-arguments.any.html": true,
"textdecoder-arguments.any.worker.html": true,
Expand Down Expand Up @@ -1162,7 +1077,9 @@
"single-byte-decoder.window.html?TextDecoder": true,
"textdecoder-eof.any.html": true,
"textdecoder-eof.any.worker.html": true,
"idlharness-shadowrealm.window.html": false
"idlharness-shadowrealm.window.html": false,
"single-byte-decoder.window.html?XMLHttpRequest": false,
"single-byte-decoder.window.html?document": false
},
"hr-time": {
"monotonic-clock.any.html": true,
Expand Down