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

Improve the performance of 'string enums' #3915

Merged
merged 14 commits into from May 21, 2024
Merged
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -31,6 +31,9 @@
* Generate JS bindings for WebIDL dictionary setters instead of using `Reflect`. This increases the size of the Web API bindings but should be more performant. Also, importing getters/setters from JS now supports specifying the JS attribute name as a string, e.g. `#[wasm_bindgen(method, setter = "x-cdm-codecs")]`.
[#3898](https://github.com/rustwasm/wasm-bindgen/pull/3898)

* Greatly improve the performance of sending WebIDL 'string enums' across the JavaScript boundary by converting the enum variant string to/from an int.
[#3915](https://github.com/rustwasm/wasm-bindgen/pull/3915)

### Fixed

* Copy port from headless test server when using `WASM_BINDGEN_TEST_ADDRESS`.
Expand Down
4 changes: 2 additions & 2 deletions benchmarks/index.html
Expand Up @@ -277,12 +277,12 @@ <h1>wasm-bindgen benchmarks</h1>

<tr>
<td>
Call a custom JS function with an enum value parameter
Call a custom JS function with a string enum value parameter

<a class='about-open' href='#'>(?)</a>

<p class='about'>
Benchmarks the speed of passing enum values to javascript
Benchmarks the speed of passing string enums to javascript
</p>
</td>

Expand Down
8 changes: 4 additions & 4 deletions crates/backend/src/ast.rs
Expand Up @@ -163,7 +163,7 @@ pub enum ImportKind {
/// Importing a type/class
Type(ImportType),
/// Importing a JS enum
Enum(ImportEnum),
Enum(StringEnum),
}

/// A function being imported from JS
Expand Down Expand Up @@ -302,10 +302,10 @@ pub struct ImportType {
pub wasm_bindgen: Path,
}

/// The metadata for an Enum being imported
/// The metadata for a String Enum
#[cfg_attr(feature = "extra-traits", derive(Debug, PartialEq, Eq))]
#[derive(Clone)]
pub struct ImportEnum {
pub struct StringEnum {
/// The Rust enum's visibility
pub vis: syn::Visibility,
/// The Rust enum's identifiers
Expand Down Expand Up @@ -404,7 +404,7 @@ pub struct StructField {
pub wasm_bindgen: Path,
}

/// Information about an Enum being exported
/// The metadata for an Enum
#[cfg_attr(feature = "extra-traits", derive(Debug, PartialEq, Eq))]
#[derive(Clone)]
pub struct Enum {
Expand Down
123 changes: 71 additions & 52 deletions crates/backend/src/codegen.rs
Expand Up @@ -3,7 +3,7 @@ use crate::encode;
use crate::encode::EncodeChunk;
use crate::Diagnostic;
use once_cell::sync::Lazy;
use proc_macro2::{Ident, Literal, Span, TokenStream};
use proc_macro2::{Ident, Span, TokenStream};
use quote::format_ident;
use quote::quote_spanned;
use quote::{quote, ToTokens};
Expand Down Expand Up @@ -1075,117 +1075,136 @@ impl ToTokens for ast::ImportType {
}
}

impl ToTokens for ast::ImportEnum {
impl ToTokens for ast::StringEnum {
fn to_tokens(&self, tokens: &mut TokenStream) {
let vis = &self.vis;
let name = &self.name;
let expect_string = format!("attempted to convert invalid {} into JSValue", name);
let enum_name = &self.name;
let name_str = enum_name.to_string();
let name_len = name_str.len() as u32;
let name_chars = name_str.chars().map(u32::from);
let variants = &self.variants;
let variant_strings = &self.variant_values;
let variant_count = self.variant_values.len() as u32;
let variant_values = &self.variant_values;
let variant_indices = (0..variant_count).collect::<Vec<_>>();
let invalid = variant_count;
let hole = variant_count + 1;
let attrs = &self.rust_attrs;

let mut current_idx: usize = 0;
let variant_indexes: Vec<Literal> = variants
.iter()
.map(|_| {
let this_index = current_idx;
current_idx += 1;
Literal::usize_unsuffixed(this_index)
})
.collect();

// Borrow variant_indexes because we need to use it multiple times inside the quote! macro
let variant_indexes_ref = &variant_indexes;
let invalid_to_str_msg = format!(
"Converting an invalid string enum ({}) back to a string is currently not supported",
enum_name
);

// A vector of EnumName::VariantName tokens for this enum
let variant_paths: Vec<TokenStream> = self
.variants
.iter()
.map(|v| quote!(#name::#v).into_token_stream())
.map(|v| quote!(#enum_name::#v).into_token_stream())
.collect();

// Borrow variant_paths because we need to use it multiple times inside the quote! macro
let variant_paths_ref = &variant_paths;

let wasm_bindgen = &self.wasm_bindgen;

let describe_variants = self.variant_values.iter().map(|variant_value| {
let length = variant_value.len() as u32;
let chars = variant_value.chars().map(u32::from);
quote! {
inform(#length);
#(inform(#chars);)*
}
});

(quote! {
#(#attrs)*
#vis enum #name {
#(#variants = #variant_indexes_ref,)*
#[non_exhaustive]
daxpedda marked this conversation as resolved.
Show resolved Hide resolved
#[repr(u32)]
#vis enum #enum_name {
#(#variants = #variant_indices,)*
#[automatically_derived]
#[doc(hidden)]
__Nonexhaustive,
__Invalid
}

#[automatically_derived]
impl #name {
fn from_str(s: &str) -> Option<#name> {
impl #enum_name {
fn from_str(s: &str) -> Option<#enum_name> {
match s {
#(#variant_strings => Some(#variant_paths_ref),)*
#(#variant_values => Some(#variant_paths_ref),)*
_ => None,
}
}

fn to_str(&self) -> &'static str {
match self {
#(#variant_paths_ref => #variant_strings,)*
#name::__Nonexhaustive => panic!(#expect_string),
#(#variant_paths_ref => #variant_values,)*
#enum_name::__Invalid => panic!(#invalid_to_str_msg),
}
}

#vis fn from_js_value(obj: &#wasm_bindgen::JsValue) -> Option<#name> {
#vis fn from_js_value(obj: &#wasm_bindgen::JsValue) -> Option<#enum_name> {
obj.as_string().and_then(|obj_str| Self::from_str(obj_str.as_str()))
}
}

// It should really be using &str for all of these, but that requires some major changes to cli-support
#[automatically_derived]
impl #wasm_bindgen::describe::WasmDescribe for #name {
fn describe() {
<#wasm_bindgen::JsValue as #wasm_bindgen::describe::WasmDescribe>::describe()
}
}

#[automatically_derived]
impl #wasm_bindgen::convert::IntoWasmAbi for #name {
type Abi = <#wasm_bindgen::JsValue as #wasm_bindgen::convert::IntoWasmAbi>::Abi;
impl #wasm_bindgen::convert::IntoWasmAbi for #enum_name {
type Abi = u32;

#[inline]
fn into_abi(self) -> Self::Abi {
<#wasm_bindgen::JsValue as #wasm_bindgen::convert::IntoWasmAbi>::into_abi(self.into())
fn into_abi(self) -> u32 {
self as u32
}
}

#[automatically_derived]
impl #wasm_bindgen::convert::FromWasmAbi for #name {
type Abi = <#wasm_bindgen::JsValue as #wasm_bindgen::convert::FromWasmAbi>::Abi;
impl #wasm_bindgen::convert::FromWasmAbi for #enum_name {
type Abi = u32;

unsafe fn from_abi(js: Self::Abi) -> Self {
let s = <#wasm_bindgen::JsValue as #wasm_bindgen::convert::FromWasmAbi>::from_abi(js);
#name::from_js_value(&s).unwrap_or(#name::__Nonexhaustive)
unsafe fn from_abi(val: u32) -> Self {
match val {
#(#variant_indices => #variant_paths_ref,)*
#invalid => #enum_name::__Invalid,
_ => unreachable!("The JS binding should only ever produce a valid value or the specific 'invalid' value"),
}
}
}

#[automatically_derived]
impl #wasm_bindgen::convert::OptionIntoWasmAbi for #name {
impl #wasm_bindgen::convert::OptionFromWasmAbi for #enum_name {
#[inline]
fn none() -> Self::Abi { <::js_sys::Object as #wasm_bindgen::convert::OptionIntoWasmAbi>::none() }
fn is_none(val: &u32) -> bool { *val == #hole }
}

#[automatically_derived]
impl #wasm_bindgen::convert::OptionFromWasmAbi for #name {
impl #wasm_bindgen::convert::OptionIntoWasmAbi for #enum_name {
#[inline]
fn is_none(abi: &Self::Abi) -> bool { <::js_sys::Object as #wasm_bindgen::convert::OptionFromWasmAbi>::is_none(abi) }
fn none() -> Self::Abi { #hole }
}

#[automatically_derived]
impl #wasm_bindgen::describe::WasmDescribe for #enum_name {
fn describe() {
use #wasm_bindgen::describe::*;
inform(STRING_ENUM);
inform(#name_len);
#(inform(#name_chars);)*
inform(#variant_count);
#(#describe_variants)*
}
}

#[automatically_derived]
impl From<#name> for #wasm_bindgen::JsValue {
fn from(obj: #name) -> #wasm_bindgen::JsValue {
#wasm_bindgen::JsValue::from(obj.to_str())
impl #wasm_bindgen::__rt::core::convert::From<#enum_name> for
#wasm_bindgen::JsValue
{
fn from(val: #enum_name) -> Self {
#wasm_bindgen::JsValue::from_str(val.to_str())
}
}
}).to_tokens(tokens);
})
.to_tokens(tokens);
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/backend/src/encode.rs
Expand Up @@ -339,8 +339,8 @@ fn shared_import_type<'a>(i: &'a ast::ImportType, intern: &'a Interner) -> Impor
}
}

fn shared_import_enum<'a>(_i: &'a ast::ImportEnum, _intern: &'a Interner) -> ImportEnum {
ImportEnum {}
fn shared_import_enum<'a>(_i: &'a ast::StringEnum, _intern: &'a Interner) -> StringEnum {
daxpedda marked this conversation as resolved.
Show resolved Hide resolved
StringEnum {}
}

fn shared_struct<'a>(s: &'a ast::Struct, intern: &'a Interner) -> Struct<'a> {
Expand Down
25 changes: 24 additions & 1 deletion crates/cli-support/src/descriptor.rs
Davidster marked this conversation as resolved.
Show resolved Hide resolved
Expand Up @@ -34,6 +34,7 @@ tys! {
EXTERNREF
NAMED_EXTERNREF
ENUM
STRING_ENUM
RUST_STRUCT
CHAR
OPTIONAL
Expand Down Expand Up @@ -67,7 +68,16 @@ pub enum Descriptor {
String,
Externref,
NamedExternref(String),
Enum { name: String, hole: u32 },
Enum {
name: String,
hole: u32,
},
StringEnum {
name: String,
invalid: u32,
hole: u32,
Davidster marked this conversation as resolved.
Show resolved Hide resolved
variant_values: Vec<String>,
},
RustStruct(String),
Char,
Option(Box<Descriptor>),
Expand Down Expand Up @@ -156,6 +166,19 @@ impl Descriptor {
let hole = get(data);
Descriptor::Enum { name, hole }
}
STRING_ENUM => {
let name = get_string(data);
let variant_count = get(data);
let invalid = variant_count;
let hole = variant_count + 1;
let variant_values = (0..variant_count).map(|_| get_string(data)).collect();
Descriptor::StringEnum {
name,
invalid,
hole,
variant_values,
}
}
RUST_STRUCT => {
let name = get_string(data);
Descriptor::RustStruct(name)
Expand Down