Improve the performance of 'string enums' (rustwasm#3915)

CHANGELOG.md

@@ -13,6 +13,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
 
 * Fix `catch` not being thread-safe.

benchmarks/index.html

@@ -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>
 

crates/backend/src/ast.rs

@@ -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
@@ -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
@@ -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 {

crates/backend/src/codegen.rs

@@ -2,7 +2,7 @@ use crate::ast;
 use crate::encode;
 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};
@@ -1015,117 +1015,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]
+            #[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);
     }
 }
 

crates/backend/src/encode.rs

@@ -325,8 +325,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 {
+    StringEnum {}
 }
 
 fn shared_struct<'a>(s: &'a ast::Struct, intern: &'a Interner) -> Struct<'a> {

crates/cli-support/src/descriptor.rs

@@ -34,6 +34,7 @@ tys! {
     EXTERNREF
     NAMED_EXTERNREF
     ENUM
+    STRING_ENUM
     RUST_STRUCT
     CHAR
     OPTIONAL
@@ -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,
+        variant_values: Vec<String>,
+    },
     RustStruct(String),
     Char,
     Option(Box<Descriptor>),
@@ -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)