From 4b7d306a198d020ce2b6fa1c758c71714bfd036c Mon Sep 17 00:00:00 2001 From: Aaron O'Mullan Date: Sun, 24 Apr 2022 09:28:46 -0300 Subject: [PATCH] perf(serde_v8): zero-copy StringOrBuffer (#14381) --- Cargo.lock | 1 - ext/http/lib.rs | 7 +-- serde_v8/Cargo.toml | 1 - serde_v8/benches/de.rs | 54 +++++++++++++++++++++ serde_v8/de.rs | 7 ++- serde_v8/magic/buffer.rs | 7 +++ serde_v8/magic/string_or_buffer.rs | 75 ++++++++++++++++-------------- serde_v8/ser.rs | 13 +++++- serde_v8/tests/de.rs | 8 ++-- 9 files changed, 127 insertions(+), 46 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b60e7bb6d3..5ae7b509cb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3568,7 +3568,6 @@ dependencies = [ "bencher", "derive_more", "serde", - "serde_bytes", "serde_json", "v8", ] diff --git a/ext/http/lib.rs b/ext/http/lib.rs index 7fc90843f9..4911c543b9 100644 --- a/ext/http/lib.rs +++ b/ext/http/lib.rs @@ -38,6 +38,7 @@ use deno_websocket::ws_create_server_stream; use flate2::write::GzEncoder; use flate2::Compression; use fly_accept_encoding::Encoding; +use hyper::body::Bytes; use hyper::server::conn::Http; use hyper::service::Service; use hyper::Body; @@ -612,7 +613,7 @@ async fn op_http_write_headers( // (~4MB) let mut writer = brotli::CompressorWriter::new(Vec::new(), 4096, 6, 22); - writer.write_all(&data.into_bytes())?; + writer.write_all(&data)?; body = builder.body(writer.into_inner().into())?; } _ => { @@ -623,14 +624,14 @@ async fn op_http_write_headers( // 1. // https://nginx.org/en/docs/http/ngx_http_gzip_module.html#gzip_comp_level let mut writer = GzEncoder::new(Vec::new(), Compression::new(1)); - writer.write_all(&data.into_bytes())?; + writer.write_all(&data)?; body = builder.body(writer.finish()?.into())?; } } } else { // If a buffer was passed, but isn't compressible, we use it to // construct a response body. - body = builder.body(data.into_bytes().into())?; + body = builder.body(Bytes::copy_from_slice(&data).into())?; } new_wr = HttpResponseWriter::Closed; } diff --git a/serde_v8/Cargo.toml b/serde_v8/Cargo.toml index 57f3253076..030950b4d7 100644 --- a/serde_v8/Cargo.toml +++ b/serde_v8/Cargo.toml @@ -15,7 +15,6 @@ path = "lib.rs" [dependencies] derive_more = "0.99.17" serde = { version = "1.0.130", features = ["derive"] } -serde_bytes = "0.11" v8 = "0.42.0" [dev-dependencies] diff --git a/serde_v8/benches/de.rs b/serde_v8/benches/de.rs index 23b1161c78..72e5799cd5 100644 --- a/serde_v8/benches/de.rs +++ b/serde_v8/benches/de.rs @@ -155,6 +155,54 @@ fn de_bstr_v8_1024_b(b: &mut Bencher) { ); } +fn de_sob_str_6b(b: &mut Bencher) { + dedo("'byebye'", |scope, v| { + b.iter(move || { + let _: serde_v8::StringOrBuffer = serde_v8::from_v8(scope, v).unwrap(); + }) + }) +} + +fn de_sob_str_1kb(b: &mut Bencher) { + dedo("'deno'.repeat(256)", |scope, v| { + b.iter(move || { + let _: serde_v8::StringOrBuffer = serde_v8::from_v8(scope, v).unwrap(); + }) + }) +} + +fn de_sob_buf_1b(b: &mut Bencher) { + dedo("new Uint8Array([97])", |scope, v| { + b.iter(move || { + let _: serde_v8::StringOrBuffer = serde_v8::from_v8(scope, v).unwrap(); + }) + }) +} + +fn de_sob_buf_1kb(b: &mut Bencher) { + dedo("(new Uint8Array(1*1024)).fill(42)", |scope, v| { + b.iter(move || { + let _: serde_v8::StringOrBuffer = serde_v8::from_v8(scope, v).unwrap(); + }) + }) +} + +fn de_sob_buf_16kb(b: &mut Bencher) { + dedo("(new Uint8Array(16*1024)).fill(42)", |scope, v| { + b.iter(move || { + let _: serde_v8::StringOrBuffer = serde_v8::from_v8(scope, v).unwrap(); + }) + }) +} + +fn de_sob_buf_512kb(b: &mut Bencher) { + dedo("(new Uint8Array(512*1024)).fill(42)", |scope, v| { + b.iter(move || { + let _: serde_v8::StringOrBuffer = serde_v8::from_v8(scope, v).unwrap(); + }) + }) +} + benchmark_group!( benches, de_struct_v8, @@ -174,6 +222,12 @@ benchmark_group!( de_tuple_v8_opt, de_bstr_v8_12_b, de_bstr_v8_1024_b, + de_sob_str_6b, + de_sob_str_1kb, + de_sob_buf_1b, + de_sob_buf_1kb, + de_sob_buf_16kb, + de_sob_buf_512kb, ); benchmark_main!(benches); diff --git a/serde_v8/de.rs b/serde_v8/de.rs index fb8b8edcfc..54a3ffa5c5 100644 --- a/serde_v8/de.rs +++ b/serde_v8/de.rs @@ -7,7 +7,9 @@ use crate::keys::{v8_struct_key, KeyCache}; use crate::magic::transl8::FromV8; use crate::magic::transl8::{visit_magic, MagicType}; use crate::payload::ValueType; -use crate::{magic, Buffer, ByteString, DetachedBuffer, U16String}; +use crate::{ + magic, Buffer, ByteString, DetachedBuffer, StringOrBuffer, U16String, +}; pub struct Deserializer<'a, 'b, 's> { input: v8::Local<'a, v8::Value>, @@ -359,6 +361,9 @@ impl<'de, 'a, 'b, 's, 'x> de::Deserializer<'de> U16String::MAGIC_NAME => { visit_magic(visitor, U16String::from_v8(self.scope, self.input)?) } + StringOrBuffer::MAGIC_NAME => { + visit_magic(visitor, StringOrBuffer::from_v8(self.scope, self.input)?) + } magic::Value::MAGIC_NAME => { visit_magic(visitor, magic::Value::from_v8(self.scope, self.input)?) } diff --git a/serde_v8/magic/buffer.rs b/serde_v8/magic/buffer.rs index 3a8c9499b8..dfa6928fbf 100644 --- a/serde_v8/magic/buffer.rs +++ b/serde_v8/magic/buffer.rs @@ -1,5 +1,6 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. +use std::fmt::Debug; use std::ops::Deref; use std::ops::DerefMut; use std::sync::Mutex; @@ -21,6 +22,12 @@ pub enum MagicBuffer { impl_magic!(MagicBuffer); +impl Debug for MagicBuffer { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_list().entries(self.as_ref().iter()).finish() + } +} + impl MagicBuffer { pub fn empty() -> Self { MagicBuffer::ToV8(Mutex::new(Some(vec![0_u8; 0].into_boxed_slice()))) diff --git a/serde_v8/magic/string_or_buffer.rs b/serde_v8/magic/string_or_buffer.rs index edde2adcd1..e04cb1a93f 100644 --- a/serde_v8/magic/string_or_buffer.rs +++ b/serde_v8/magic/string_or_buffer.rs @@ -1,45 +1,50 @@ +// Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. +use super::buffer::MagicBuffer; +use super::transl8::{FromV8, ToV8}; +use crate::magic::transl8::impl_magic; +use crate::Error; use std::ops::Deref; #[derive(Debug)] -pub struct StringOrBuffer(Vec); - -impl Deref for StringOrBuffer { - type Target = Vec; - fn deref(&self) -> &Vec { - &self.0 - } -} - -impl StringOrBuffer { - pub fn into_bytes(self) -> Vec { - self.0 - } -} - -impl<'de> serde::Deserialize<'de> for StringOrBuffer { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - StringOrBufferInner::deserialize(deserializer) - .map(|x| StringOrBuffer(x.into_bytes())) - } -} - -// TODO(@AaronO): explore if we can make this work with ZeroCopyBuf -#[derive(serde::Deserialize)] -#[serde(untagged)] -enum StringOrBufferInner { - #[serde(with = "serde_bytes")] - Buffer(Vec), +pub enum StringOrBuffer { + Buffer(MagicBuffer), String(String), } -impl StringOrBufferInner { - fn into_bytes(self) -> Vec { +impl_magic!(StringOrBuffer); + +impl Deref for StringOrBuffer { + type Target = [u8]; + fn deref(&self) -> &Self::Target { match self { - Self::String(s) => s.into_bytes(), - Self::Buffer(b) => b, + Self::Buffer(b) => b.as_ref(), + Self::String(s) => s.as_bytes(), } } } + +impl ToV8 for StringOrBuffer { + fn to_v8<'a>( + &self, + scope: &mut v8::HandleScope<'a>, + ) -> Result, crate::Error> { + match self { + Self::Buffer(buf) => crate::to_v8(scope, buf), + Self::String(s) => crate::to_v8(scope, s), + } + } +} + +impl FromV8 for StringOrBuffer { + fn from_v8( + scope: &mut v8::HandleScope, + value: v8::Local, + ) -> Result { + if let Ok(buf) = MagicBuffer::from_v8(scope, value) { + return Ok(Self::Buffer(buf)); + } else if let Ok(s) = crate::from_v8(scope, value) { + return Ok(Self::String(s)); + } + Err(Error::ExpectedBuffer) + } +} diff --git a/serde_v8/ser.rs b/serde_v8/ser.rs index 867d4b7952..eb6fc8e70c 100644 --- a/serde_v8/ser.rs +++ b/serde_v8/ser.rs @@ -8,7 +8,9 @@ use crate::error::{Error, Result}; use crate::keys::v8_struct_key; use crate::magic::transl8::MAGIC_FIELD; use crate::magic::transl8::{opaque_deref, opaque_recv, MagicType, ToV8}; -use crate::{magic, Buffer, ByteString, DetachedBuffer, U16String}; +use crate::{ + magic, Buffer, ByteString, DetachedBuffer, StringOrBuffer, U16String, +}; type JsValue<'s> = v8::Local<'s, v8::Value>; type JsResult<'s> = Result>; @@ -264,6 +266,7 @@ pub enum StructSerializers<'a, 'b, 'c> { MagicDetached(MagicalSerializer<'a, 'b, 'c, DetachedBuffer>), MagicByteString(MagicalSerializer<'a, 'b, 'c, ByteString>), MagicU16String(MagicalSerializer<'a, 'b, 'c, U16String>), + MagicStringOrBuffer(MagicalSerializer<'a, 'b, 'c, StringOrBuffer>), Regular(ObjectSerializer<'a, 'b, 'c>), } @@ -282,6 +285,9 @@ impl<'a, 'b, 'c> ser::SerializeStruct for StructSerializers<'a, 'b, 'c> { StructSerializers::MagicDetached(s) => s.serialize_field(key, value), StructSerializers::MagicByteString(s) => s.serialize_field(key, value), StructSerializers::MagicU16String(s) => s.serialize_field(key, value), + StructSerializers::MagicStringOrBuffer(s) => { + s.serialize_field(key, value) + } StructSerializers::Regular(s) => s.serialize_field(key, value), } } @@ -293,6 +299,7 @@ impl<'a, 'b, 'c> ser::SerializeStruct for StructSerializers<'a, 'b, 'c> { StructSerializers::MagicDetached(s) => s.end(), StructSerializers::MagicByteString(s) => s.end(), StructSerializers::MagicU16String(s) => s.end(), + StructSerializers::MagicStringOrBuffer(s) => s.end(), StructSerializers::Regular(s) => s.end(), } } @@ -535,6 +542,10 @@ impl<'a, 'b, 'c> ser::Serializer for Serializer<'a, 'b, 'c> { let m = MagicalSerializer::::new(self.scope); Ok(StructSerializers::MagicDetached(m)) } + StringOrBuffer::MAGIC_NAME => { + let m = MagicalSerializer::::new(self.scope); + Ok(StructSerializers::MagicStringOrBuffer(m)) + } magic::Value::MAGIC_NAME => { let m = MagicalSerializer::>::new(self.scope); Ok(StructSerializers::Magic(m)) diff --git a/serde_v8/tests/de.rs b/serde_v8/tests/de.rs index c6c4451f76..6eea680c6a 100644 --- a/serde_v8/tests/de.rs +++ b/serde_v8/tests/de.rs @@ -178,24 +178,24 @@ fn de_map() { fn de_string_or_buffer() { dedo("'hello'", |scope, v| { let sob: serde_v8::StringOrBuffer = serde_v8::from_v8(scope, v).unwrap(); - assert_eq!(sob.as_slice(), &[0x68, 0x65, 0x6C, 0x6C, 0x6F]); + assert_eq!(sob.as_ref(), &[0x68, 0x65, 0x6C, 0x6C, 0x6F]); }); dedo("new Uint8Array([97])", |scope, v| { let sob: serde_v8::StringOrBuffer = serde_v8::from_v8(scope, v).unwrap(); - assert_eq!(sob.as_slice(), &[97]); + assert_eq!(sob.as_ref(), &[97]); }); dedo("new Uint8Array([128])", |scope, v| { let sob: serde_v8::StringOrBuffer = serde_v8::from_v8(scope, v).unwrap(); - assert_eq!(sob.as_slice(), &[128]); + assert_eq!(sob.as_ref(), &[128]); }); dedo( "(Uint8Array.from([0x68, 0x65, 0x6C, 0x6C, 0x6F]))", |scope, v| { let sob: serde_v8::StringOrBuffer = serde_v8::from_v8(scope, v).unwrap(); - assert_eq!(sob.as_slice(), &[0x68, 0x65, 0x6C, 0x6C, 0x6F]); + assert_eq!(sob.as_ref(), &[0x68, 0x65, 0x6C, 0x6C, 0x6F]); }, ); }