Improve the performance of WebIDL 'import enums' #3915
Conversation
Davidster
changed the title
Davidster
changed the title
Davidster
changed the title
crates/backend/src/codegen.rs
Outdated
| unsafe fn from_abi(val: u32) -> Self { | ||
| match val { | ||
| #(#variant_indices => #variant_paths_ref,)* | ||
| _ => Self::__Invalid |
There was a problem hiding this comment.
Returning __Invalid here preserves the current behaviour and allows the invalid_enum_return test to pass. Is this the approach we want to take or would panicking be better?
| (quote! { | ||
| #(#attrs)* | ||
| #vis enum #name { | ||
| #(#variants = #variant_indexes_ref,)* | ||
| #[non_exhaustive] |
There was a problem hiding this comment.
Should we explore removing the non_exhaustive here?
|
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; |
|
I'm not sure what's going on with the CI. The |
|
Apologies for the delay, I was on vacation, still catching up.
Run with |
|
No worries! Thanks I missed the flag |
…r the large describe functions generated by import enums
| ret.mem = vec![0; 0x400]; | ||
| // Give ourselves some memory and set the stack pointer | ||
| // (the LLVM call stack, now the wasm stack, global 0) to the top. | ||
| ret.mem = vec![0; 0x8000]; |
There was a problem hiding this comment.
So this was... fun to debug
The wasm interpreter/runtime that's initialized here holds onto a stack pointer that starts at the highest memory location mem.len(), which was 0x400 originally (1024). Then, as the program runs and local variables are pushed onto the stack the stack pointer is decremented for each new variable. The other changes in this PR increased the complexity of some of the describes by a large amount, and so the stack pointer eventually got decremented so many times in one of those functions that it fell below 0 which is illegal (a.k.a stack overflow). So I increased the program memory up to 0x2000 (8192) and then the crash stopped happening.
In particular it was the GpuTextureFormat enum that was causing the problem due to the number of enum variants it has (and therefore the number of instructions required to implement the describe function). Its describe function seemed to need around 5kb of stack space.
This post helped me understand this issue: https://hackmd.io/@pepyakin/SkmPKGhiq
Closes #3468