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

Conversation

Davidster
Copy link
Contributor

@Davidster Davidster commented Apr 5, 2024

Closes #3468

@Davidster Davidster changed the title Import enum binding rework Greatly improve the performance of sending WebIDL 'import enums' across the JavaScript boundary bindings by converting the enum variant string to/from an int Apr 5, 2024
@Davidster Davidster changed the title Greatly improve the performance of sending WebIDL 'import enums' across the JavaScript boundary bindings by converting the enum variant string to/from an int Greatly improve the performance of WebIDL 'import enums' Apr 5, 2024
@Davidster Davidster changed the title Greatly improve the performance of WebIDL 'import enums' Improve the performance of WebIDL 'import enums' Apr 5, 2024
@Davidster
Copy link
Contributor Author

For your convenience, this is what a fully-expanded ImportEnum looks like now:

#![feature(prelude_import)]
//! Raw API bindings for Web APIs
//!
//! This is a procedurally generated crate from browser WebIDL which provides a
//! binding to all APIs that browsers provide on the web.
//!
//! This crate by default contains very little when compiled as almost all of
//! its exposed APIs are gated by Cargo features. The exhaustive list of
//! features can be found in `crates/web-sys/Cargo.toml`, but the rule of thumb
//! for `web-sys` is that each type has its own cargo feature (named after the
//! type). Using an API requires enabling the features for all types used in the
//! API, and APIs should mention in the documentation what features they
//! require.
#![doc(html_root_url = "https://docs.rs/web-sys/0.3")]
#![allow(deprecated)]
#[prelude_import]
use std::prelude::rust_2018::*;
#[macro_use]
extern crate std;
mod features {
    #[cfg(feature = "RequestMode")]
    #[allow(non_snake_case)]
    mod gen_RequestMode {
        #![allow(unused_imports)]
        #![allow(clippy::all)]
        use wasm_bindgen::prelude::*;
        ///The `RequestMode` enum.
        ///
        ///*This API requires the following crate features to be activated: `RequestMode`*
        #[non_exhaustive]
        #[repr(u32)]
        pub enum RequestMode {
            SameOrigin = 0u32,
            NoCors = 1u32,
            Cors = 2u32,
            Navigate = 3u32,
            #[automatically_derived]
            #[doc(hidden)]
            __Invalid,
        }
        #[automatically_derived]
        impl ::core::fmt::Debug for RequestMode {
            #[inline]
            fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
                ::core::fmt::Formatter::write_str(
                    f,
                    match self {
                        RequestMode::SameOrigin => "SameOrigin",
                        RequestMode::NoCors => "NoCors",
                        RequestMode::Cors => "Cors",
                        RequestMode::Navigate => "Navigate",
                        RequestMode::__Invalid => "__Invalid",
                    },
                )
            }
        }
        #[automatically_derived]
        impl ::core::clone::Clone for RequestMode {
            #[inline]
            fn clone(&self) -> RequestMode {
                *self
            }
        }
        #[automatically_derived]
        impl ::core::marker::Copy for RequestMode {}
        #[automatically_derived]
        impl ::core::marker::StructuralPartialEq for RequestMode {}
        #[automatically_derived]
        impl ::core::cmp::PartialEq for RequestMode {
            #[inline]
            fn eq(&self, other: &RequestMode) -> bool {
                let __self_tag = ::core::intrinsics::discriminant_value(self);
                let __arg1_tag = ::core::intrinsics::discriminant_value(other);
                __self_tag == __arg1_tag
            }
        }
        #[automatically_derived]
        impl ::core::marker::StructuralEq for RequestMode {}
        #[automatically_derived]
        impl ::core::cmp::Eq for RequestMode {
            #[inline]
            #[doc(hidden)]
            #[coverage(off)]
            fn assert_receiver_is_total_eq(&self) -> () {}
        }
        #[automatically_derived]
        impl wasm_bindgen::convert::IntoWasmAbi for RequestMode {
            type Abi = u32;
            #[inline]
            fn into_abi(self) -> u32 {
                self as u32
            }
        }
        #[automatically_derived]
        impl wasm_bindgen::convert::FromWasmAbi for RequestMode {
            type Abi = u32;
            unsafe fn from_abi(val: u32) -> Self {
                match val {
                    0u32 => RequestMode::SameOrigin,
                    1u32 => RequestMode::NoCors,
                    2u32 => RequestMode::Cors,
                    3u32 => RequestMode::Navigate,
                    4u32 => RequestMode::__Invalid,
                    _ => {
                        ::core::panicking::unreachable_display(
                            &"The JS binding should only ever produce a valid value or the specific 'invalid' value",
                        );
                    }
                }
            }
        }
        #[automatically_derived]
        impl wasm_bindgen::convert::OptionFromWasmAbi for RequestMode {
            #[inline]
            fn is_none(val: &u32) -> bool {
                *val == 5u32
            }
        }
        #[automatically_derived]
        impl wasm_bindgen::convert::OptionIntoWasmAbi for RequestMode {
            #[inline]
            fn none() -> Self::Abi {
                5u32
            }
        }
        #[automatically_derived]
        impl wasm_bindgen::describe::WasmDescribe for RequestMode {
            fn describe() {
                use wasm_bindgen::describe::*;
                inform(IMPORT_ENUM);
                inform(11u32);
                inform(82u32);
                inform(101u32);
                inform(113u32);
                inform(117u32);
                inform(101u32);
                inform(115u32);
                inform(116u32);
                inform(77u32);
                inform(111u32);
                inform(100u32);
                inform(101u32);
                inform(4u32);
                inform(5u32);
                inform(4u32);
                inform(11u32);
                inform(115u32);
                inform(97u32);
                inform(109u32);
                inform(101u32);
                inform(45u32);
                inform(111u32);
                inform(114u32);
                inform(105u32);
                inform(103u32);
                inform(105u32);
                inform(110u32);
                inform(7u32);
                inform(110u32);
                inform(111u32);
                inform(45u32);
                inform(99u32);
                inform(111u32);
                inform(114u32);
                inform(115u32);
                inform(4u32);
                inform(99u32);
                inform(111u32);
                inform(114u32);
                inform(115u32);
                inform(8u32);
                inform(110u32);
                inform(97u32);
                inform(118u32);
                inform(105u32);
                inform(103u32);
                inform(97u32);
                inform(116u32);
                inform(101u32);
            }
        }
        #[automatically_derived]
        impl wasm_bindgen::__rt::core::convert::From<RequestMode>
        for wasm_bindgen::JsValue {
            fn from(val: RequestMode) -> Self {
                wasm_bindgen::JsValue::from_str(
                    match val {
                        RequestMode::SameOrigin => "same-origin",
                        RequestMode::NoCors => "no-cors",
                        RequestMode::Cors => "cors",
                        RequestMode::Navigate => "navigate",
                        RequestMode::__Invalid => {
                            wasm_bindgen::throw_str(
                                "Converting an invalid import enum back to a string is currently not supported",
                            )
                        }
                        _ => {
                            ::core::panicking::unreachable_display(
                                &"All possible variants should have been checked",
                            );
                        }
                    },
                )
            }
        }
    }
    #[cfg(feature = "RequestMode")]
    #[allow(unused_imports)]
    pub use gen_RequestMode::*;
}
pub use features::*;
pub use js_sys;
pub use wasm_bindgen;

@Davidster
Copy link
Contributor Author

Davidster commented Apr 5, 2024

I'm not sure what's going on with the CI. The cargo test --manifest-path crates/web-sys/Cargo.toml --target wasm32-unknown-unknown --all-features test passes locally for me. Any ideas?

@daxpedda
Copy link
Collaborator

Apologies for the delay, I was on vacation, still catching up.
Planning to take a look tomorrow!

I'm not sure what's going on with the CI. The cargo test --manifest-path crates/web-sys/Cargo.toml --target wasm32-unknown-unknown --all-features test passes locally for me. Any ideas?

Run with RUSTFLAGS=--cfg=web_sys_unstable_apis, which reproduced the bug for me locally as well.

@Davidster
Copy link
Contributor Author

No worries! Thanks I missed the flag

…r the large describe functions generated by import enums
Copy link

@MartensCedric MartensCedric left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@adrientremblay adrientremblay left a comment

Choose a reason for hiding this comment

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

Good job

Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

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

Looks great so far, basically just a couple of nits.

Again, I apologize for the delay, I extended my vacation a bit spontaneously.

crates/backend/src/codegen.rs Outdated Show resolved Hide resolved
crates/backend/src/codegen.rs Outdated Show resolved Hide resolved
crates/backend/src/codegen.rs Show resolved Hide resolved
crates/backend/src/codegen.rs Outdated Show resolved Hide resolved
crates/backend/src/codegen.rs Outdated Show resolved Hide resolved
crates/backend/src/codegen.rs Outdated Show resolved Hide resolved
crates/backend/src/codegen.rs Outdated Show resolved Hide resolved
crates/cli-support/src/descriptor.rs Show resolved Hide resolved
crates/cli-support/src/descriptor.rs Show resolved Hide resolved
@Davidster Davidster requested a review from daxpedda May 20, 2024 03:18
crates/backend/src/codegen.rs Outdated Show resolved Hide resolved
crates/backend/src/codegen.rs Outdated Show resolved Hide resolved
crates/backend/src/codegen.rs Outdated Show resolved Hide resolved
crates/backend/src/codegen.rs Show resolved Hide resolved
crates/backend/src/codegen.rs Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
- re-add removed to_str function
- rename 'import enum' to 'string enum'
- remove 'invalid' number from describe function
Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

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

LGTM!

@Liamolucko would appreciate your review/approval as well!

crates/shared/src/schema_hash_approval.rs Show resolved Hide resolved
@daxpedda daxpedda requested a review from Liamolucko May 20, 2024 18:29
@Davidster
Copy link
Contributor Author

thanks for the review!

@Davidster Davidster changed the title Improve the performance of WebIDL 'import enums' Improve the performance of 'string enums' May 20, 2024
Copy link
Collaborator

@Liamolucko Liamolucko left a comment

Choose a reason for hiding this comment

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

Looks great!

crates/backend/src/encode.rs Show resolved Hide resolved
@daxpedda daxpedda merged commit 9b37613 into rustwasm:main May 21, 2024
25 checks passed
@daxpedda
Copy link
Collaborator

@Davidster thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Interning internal enum representations and field names
6 participants